Author: Zeyi Xu Date: 2026-06-04T16:29:06+08:00 New Revision: b0cb331995d494dda75d5fd967e160ec0c32caf1
URL: https://github.com/llvm/llvm-project/commit/b0cb331995d494dda75d5fd967e160ec0c32caf1 DIFF: https://github.com/llvm/llvm-project/commit/b0cb331995d494dda75d5fd967e160ec0c32caf1.diff LOG: [clang-tidy] Avoid brace fix-it crash in macro body expansion (#198788) `readability-braces-around-statements `could assert when diagnosing an unbraced statement that ends in the middle of a macro body expansion. It would be hard/unsafe to give fix-its for such cases, so treat them as diagnostic-only. Closes https://github.com/llvm/llvm-project/issues/198711 Added: Modified: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-same-line.cpp clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp index 1864dfcbd29c2..710b538317626 100644 --- a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp @@ -154,7 +154,8 @@ bool BracesAroundStatementsCheck::checkStmt( S, Result.Context->getLangOpts(), *Result.SourceManager, StartLoc, EndLocHint); if (BraceInsertionHints) { - if (ShortStatementLines && !ForceBracesStmts.erase(S) && + if (ShortStatementLines && BraceInsertionHints.offersFixIts() && + !ForceBracesStmts.erase(S) && BraceInsertionHints.resultingCompoundLineExtent(*Result.SourceManager) < ShortStatementLines) return false; diff --git a/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp index d51bfd9362eb7..151fec67228c7 100644 --- a/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp +++ b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp @@ -110,7 +110,9 @@ BraceInsertionHints getBraceInsertionsHints(const Stmt *const S, // 2) If there's a multi-line block comment starting on the same line after // the location we're inserting the closing brace at, or there's a non-comment // token, the check inserts "\n}" right before that token. - // 3) Otherwise the check finds the end of line (possibly after some block or + // 3) If the statement ends in the middle of a macro body expansion, the check + // emits a diagnostic without a fix-it. + // 4) Otherwise the check finds the end of line (possibly after some block or // line comments) and inserts "\n}" right before that EOL. if (!S || isa<CompoundStmt>(S)) { // Already inside braces. @@ -157,6 +159,12 @@ BraceInsertionHints getBraceInsertionsHints(const Stmt *const S, EndLoc = EndLocHint; ClosingInsertion = "} "; } else { + const SourceLocation StmtEndLoc = S->getEndLoc(); + if (StmtEndLoc.isMacroID() && SM.isMacroBodyExpansion(StmtEndLoc) && + !Lexer::isAtEndOfMacroExpansion(StmtEndLoc, SM, LangOpts)) { + // No safe fix-it for a statement ending in the middle of a macro body. + return {StartLoc}; + } EndLoc = findEndLocation(*S, SM, LangOpts); ClosingInsertion = "\n}"; } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 71ddb9bcbe497..53d28b439c97b 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -672,6 +672,11 @@ Changes in existing checks false positives when a class is seen through both a header include and a C++20 module import. +- Improved :doc:`readability-braces-around-statements + <clang-tidy/checks/readability/braces-around-statements>` check by fixing a + crash when diagnosing a statement that ends in the middle of a macro body + expansion. + - Improved :doc:`readability-container-size-empty <clang-tidy/checks/readability/container-size-empty>` check: diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-same-line.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-same-line.cpp index 50456c6de0c89..0b2cf6e3fb072 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-same-line.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-same-line.cpp @@ -2,6 +2,10 @@ void do_something(const char *) {} +#define BAD_MACRO(x) \ + do_something(x); \ + do_something(x) + bool cond(const char *) { return false; } @@ -33,4 +37,8 @@ void test() { // CHECK-MESSAGES: :[[@LINE-7]]:31: warning: statement should be inside braces // CHECK-FIXES: if (cond("if4") /*comment*/) { // CHECK-FIXES: } + + if (cond("macro")) + BAD_MACRO("macro"); + // CHECK-MESSAGES: :[[@LINE-2]]:21: warning: statement should be inside braces } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements.cpp index f1bb4ed988d7d..6f15500040905 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements.cpp @@ -415,6 +415,14 @@ int test_macros(bool b) { // CHECK-FIXES-NEXT: do_something("for in wrapping macro 3") // CHECK-FIXES-NEXT: ); + #define BAD_MACRO(x) \ + do_something(x); \ + do_something(x) + + if (b) + BAD_MACRO("macro body"); + // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should be inside braces + // Taken from https://bugs.llvm.org/show_bug.cgi?id=22785 int i; #define MACRO_1 i++ _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
