https://github.com/AayushMainali-Github updated https://github.com/llvm/llvm-project/pull/219634
>From 06bffe0f3253ea1e2072d6a967b45fce3e2a4748 Mon Sep 17 00:00:00 2001 From: AayushMainali-Github <[email protected]> Date: Sat, 29 Aug 2026 05:59:25 +0000 Subject: [PATCH 1/4] [clang-tidy] Fix readability-trailing-comma false positive on #endif readability-trailing-comma found the token immediately before an enum's closing brace and treated it as the last enumerator or trailing comma. When enumerators were wrapped in #ifdef / #endif, that token was the directive identifier endif, so the check warned and inserted a comma after #endif even when every enumerator already had a trailing comma. Skip the diagnostic when the token before '}' is a preprocessor directive. An enumerator that is actually named endif is still diagnosed, because it is not preceded by '#'. Fixes #218957 --- .../readability/TrailingCommaCheck.cpp | 28 +++++++++++++-- .../readability/trailing-comma-cxx11.cpp | 14 ++++++++ .../checkers/readability/trailing-comma.cpp | 34 +++++++++++++++++++ 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp index cb1a33ba09233..60fa53ddf6556 100644 --- a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp @@ -56,6 +56,19 @@ AST_MATCHER(EnumDecl, isEmptyEnum) { return Node.enumerators().empty(); } AST_MATCHER(InitListExpr, isEmptyInitList) { return Node.getNumInits() == 0; } +// True when Tok is a preprocessor directive (the '#' or the directive +// identifier such as 'endif'). Those tokens can sit between the last +// enumerator and '}', and must not be treated as a missing trailing comma. +static bool isPreprocessorDirectiveToken(const Token &Tok, + const SourceManager &SM, + const LangOptions &LangOpts) { + if (Tok.is(tok::hash)) + return true; + const std::optional<Token> Prev = Lexer::findPreviousToken( + Tok.getLocation(), SM, LangOpts, /*IncludeComments=*/false); + return Prev && Prev->is(tok::hash); +} + } // namespace TrailingCommaCheck::TrailingCommaCheck(StringRef Name, @@ -110,12 +123,21 @@ void TrailingCommaCheck::checkEnumDecl(const EnumDecl *Enum, if (Policy == CommaPolicyKind::Ignore) return; - const std::optional<Token> LastTok = - Lexer::findPreviousToken(Enum->getBraceRange().getEnd(), - *Result.SourceManager, getLangOpts(), false); + const std::optional<Token> LastTok = Lexer::findPreviousToken( + Enum->getBraceRange().getEnd(), *Result.SourceManager, getLangOpts(), + /*IncludeComments=*/false); if (!LastTok) return; + // `#endif` (and similar directives) can appear immediately before the + // closing brace when enumerators are guarded by `#ifdef`. Walking back from + // `}` would otherwise treat that directive as the last enumerator and insert + // a comma after it, even when every active enumerator already has a trailing + // comma. + if (isPreprocessorDirectiveToken(*LastTok, *Result.SourceManager, + getLangOpts())) + return; + emitDiag(LastTok->getLocation(), LastTok, DiagKind::Enum, Result, Policy); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx11.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx11.cpp index 9f37db2c837c3..80c88fcc2396c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx11.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx11.cpp @@ -54,3 +54,17 @@ struct PackSingle { PackSingle<int> p1; PackSingle<int, double, char> p3; + +// Preprocessor-guarded enumerators already have trailing commas; do not insert +// a comma after '#endif'. +enum class color_t : unsigned { + RED = 0, + GREEN = 1, + BLUE = 2, + CYAN = 3, +#ifdef USE_MAGENTA + LAST = CYAN, +#else + LAST = BLUE, +#endif +}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.cpp index 76fb4bbf0c37d..daef89b770716 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.cpp @@ -144,6 +144,40 @@ void nestedMultiLine() { // CHECK-FIXES-NEXT: }; } +// Preprocessor directives immediately before '}' must not be treated as the +// last enumerator. Both branches already have trailing commas; a false +// positive would insert a comma after '#endif'. +enum color_t { + COLOR_RED = 0, + COLOR_GREEN = 1, + COLOR_BLUE = 2, + COLOR_CYAN = 3, +#ifdef USE_MAGENTA + COLOR_LAST = COLOR_CYAN, +#else + COLOR_LAST = COLOR_BLUE, +#endif +}; + +enum GuardedEnumerator { + GE_A, + GE_B, +#ifdef USE_EXTRA + GE_C, +#endif +}; + +// An enumerator named 'endif' is still diagnosed; only '#endif' is ignored. +enum EndsWithEndifName { + foo, + endif +}; +// CHECK-MESSAGES: :[[@LINE-2]]:8: warning: enum should have a trailing comma +// CHECK-FIXES: enum EndsWithEndifName { +// CHECK-FIXES-NEXT: foo, +// CHECK-FIXES-NEXT: endif, +// CHECK-FIXES-NEXT: }; + // Macros are ignored #define ENUM(n, a, b) enum n { a, b } #define INIT {1, 2} >From df6a6b0f7bf817e0f586220b6278038b4605127d Mon Sep 17 00:00:00 2001 From: AayushMainali-Github <[email protected]> Date: Sat, 29 Aug 2026 06:12:42 +0000 Subject: [PATCH 2/4] [clang-tidy] Add release note and fix trailing-comma lint --- .../readability/TrailingCommaCheck.cpp | 24 +++++++++---------- clang-tools-extra/docs/ReleaseNotes.md | 6 +++++ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp index 60fa53ddf6556..ec17275358498 100644 --- a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp @@ -44,18 +44,6 @@ static bool isSingleLine(SourceRange Range, const SourceManager &SM) { SM.getExpansionLineNumber(Range.getEnd()); } -namespace { - -AST_POLYMORPHIC_MATCHER(isMacro, - AST_POLYMORPHIC_SUPPORTED_TYPES(EnumDecl, - InitListExpr)) { - return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID(); -} - -AST_MATCHER(EnumDecl, isEmptyEnum) { return Node.enumerators().empty(); } - -AST_MATCHER(InitListExpr, isEmptyInitList) { return Node.getNumInits() == 0; } - // True when Tok is a preprocessor directive (the '#' or the directive // identifier such as 'endif'). Those tokens can sit between the last // enumerator and '}', and must not be treated as a missing trailing comma. @@ -69,6 +57,18 @@ static bool isPreprocessorDirectiveToken(const Token &Tok, return Prev && Prev->is(tok::hash); } +namespace { + +AST_POLYMORPHIC_MATCHER(isMacro, + AST_POLYMORPHIC_SUPPORTED_TYPES(EnumDecl, + InitListExpr)) { + return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID(); +} + +AST_MATCHER(EnumDecl, isEmptyEnum) { return Node.enumerators().empty(); } + +AST_MATCHER(InitListExpr, isEmptyInitList) { return Node.getNumInits() == 0; } + } // namespace TrailingCommaCheck::TrailingCommaCheck(StringRef Name, diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 420b7ddce20e6..a7cccaf542b54 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -219,6 +219,12 @@ infrastructure are described first, followed by tool-specific sections. for intermediate subobjects caused the trailing comma of the enclosing list to be incorrectly rewritten. +- Improved {doc}`readability-trailing-comma + <clang-tidy/checks/readability/trailing-comma>` check by ignoring + preprocessor directives such as `#endif` that appear immediately before an + enum's closing brace, which previously produced a false positive and a + fix-it that inserted a comma after the directive. + - Improved {doc}`readability-use-std-min-max <clang-tidy/checks/readability/use-std-min-max>` check by fixing spurious trailing semicolons and lost comments when the `if` body has no braces. >From eacdd3efa37149cf12e6ed544ba43787353f4839 Mon Sep 17 00:00:00 2001 From: AayushMainali-Github <[email protected]> Date: Sat, 29 Aug 2026 06:25:30 +0000 Subject: [PATCH 3/4] [clang-tidy] Merge readability-trailing-comma release notes --- clang-tools-extra/docs/ReleaseNotes.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index a7cccaf542b54..deca73438f807 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -214,16 +214,15 @@ infrastructure are described first, followed by tool-specific sections. option to allow customizing the set of ignored types. - Improved {doc}`readability-trailing-comma - <clang-tidy/checks/readability/trailing-comma>` check by fixing false - positives on designated initializers, where initializer lists synthesized - for intermediate subobjects caused the trailing comma of the enclosing - list to be incorrectly rewritten. + <clang-tidy/checks/readability/trailing-comma>` check: -- Improved {doc}`readability-trailing-comma - <clang-tidy/checks/readability/trailing-comma>` check by ignoring - preprocessor directives such as `#endif` that appear immediately before an - enum's closing brace, which previously produced a false positive and a - fix-it that inserted a comma after the directive. + - Fixed false positives on designated initializers, where initializer lists + synthesized for intermediate subobjects caused the trailing comma of the + enclosing list to be incorrectly rewritten. + + - Ignored preprocessor directives such as `#endif` that appear immediately + before an enum's closing brace, which previously produced a false positive + and a fix-it that inserted a comma after the directive. - Improved {doc}`readability-use-std-min-max <clang-tidy/checks/readability/use-std-min-max>` check by fixing spurious >From f8aeb863025eff0e2a489db0c4c906509c7b3583 Mon Sep 17 00:00:00 2001 From: AayushMainali-Github <[email protected]> Date: Fri, 4 Sep 2026 06:52:05 +0000 Subject: [PATCH 4/4] [clang-tidy] Tighten trailing-comma directive check --- .../readability/TrailingCommaCheck.cpp | 12 +++-------- .../readability/trailing-comma-cxx11.cpp | 3 +-- .../checkers/readability/trailing-comma.cpp | 20 +++++++++++++++---- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp index ec17275358498..3cb72a4c4ab4f 100644 --- a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp @@ -44,9 +44,6 @@ static bool isSingleLine(SourceRange Range, const SourceManager &SM) { SM.getExpansionLineNumber(Range.getEnd()); } -// True when Tok is a preprocessor directive (the '#' or the directive -// identifier such as 'endif'). Those tokens can sit between the last -// enumerator and '}', and must not be treated as a missing trailing comma. static bool isPreprocessorDirectiveToken(const Token &Tok, const SourceManager &SM, const LangOptions &LangOpts) { @@ -54,7 +51,9 @@ static bool isPreprocessorDirectiveToken(const Token &Tok, return true; const std::optional<Token> Prev = Lexer::findPreviousToken( Tok.getLocation(), SM, LangOpts, /*IncludeComments=*/false); - return Prev && Prev->is(tok::hash); + return Prev && Prev->is(tok::hash) && + SM.getExpansionLineNumber(Prev->getLocation()) == + SM.getExpansionLineNumber(Tok.getLocation()); } namespace { @@ -129,11 +128,6 @@ void TrailingCommaCheck::checkEnumDecl(const EnumDecl *Enum, if (!LastTok) return; - // `#endif` (and similar directives) can appear immediately before the - // closing brace when enumerators are guarded by `#ifdef`. Walking back from - // `}` would otherwise treat that directive as the last enumerator and insert - // a comma after it, even when every active enumerator already has a trailing - // comma. if (isPreprocessorDirectiveToken(*LastTok, *Result.SourceManager, getLangOpts())) return; diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx11.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx11.cpp index 80c88fcc2396c..041043b5ca183 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx11.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx11.cpp @@ -55,8 +55,7 @@ struct PackSingle { PackSingle<int> p1; PackSingle<int, double, char> p3; -// Preprocessor-guarded enumerators already have trailing commas; do not insert -// a comma after '#endif'. +// #endif before '}' is not a missing trailing comma. enum class color_t : unsigned { RED = 0, GREEN = 1, diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.cpp index daef89b770716..fb15931731b07 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.cpp @@ -144,9 +144,7 @@ void nestedMultiLine() { // CHECK-FIXES-NEXT: }; } -// Preprocessor directives immediately before '}' must not be treated as the -// last enumerator. Both branches already have trailing commas; a false -// positive would insert a comma after '#endif'. +// #endif before '}' is not a missing trailing comma. enum color_t { COLOR_RED = 0, COLOR_GREEN = 1, @@ -167,7 +165,6 @@ enum GuardedEnumerator { #endif }; -// An enumerator named 'endif' is still diagnosed; only '#endif' is ignored. enum EndsWithEndifName { foo, endif @@ -178,6 +175,21 @@ enum EndsWithEndifName { // CHECK-FIXES-NEXT: endif, // CHECK-FIXES-NEXT: }; +enum NullDirectiveBefore { +# + ND_A +}; +// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: enum should have a trailing comma +// CHECK-FIXES: enum NullDirectiveBefore { +// CHECK-FIXES-NEXT: # +// CHECK-FIXES-NEXT: ND_A, +// CHECK-FIXES-NEXT: }; + +enum NullDirectiveAfter { + ND_B, +# +}; + // Macros are ignored #define ENUM(n, a, b) enum n { a, b } #define INIT {1, 2} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
