Author: Aayush Mainali Date: 2026-09-28T13:02:08Z New Revision: 74f859b9d46e480fe43d7b904b1e80efb8a331a2
URL: https://github.com/llvm/llvm-project/commit/74f859b9d46e480fe43d7b904b1e80efb8a331a2 DIFF: https://github.com/llvm/llvm-project/commit/74f859b9d46e480fe43d7b904b1e80efb8a331a2.diff LOG: [clang-tidy] Fix readability-trailing-comma false positive on #endif (#219634) The `readability-trailing-comma` check looks at the token immediately before an enum's closing brace to decide whether a trailing comma is present. When the last enumerators are wrapped in `#ifdef` / `#else` / `#endif`, that token is the directive identifier `endif`, so the check reports a missing comma and `-fix` inserts `,` after `#endif`. That is outside the enumerator list and corrupts the source even when every enumerator already has a trailing comma. Skip the diagnostic when the token before `}` is a preprocessor directive (`#` or a token preceded by `#`). An enumerator whose name happens to be `endif` is still diagnosed, because it is not preceded by `#`. Fixes #218957 Added: Modified: clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp clang-tools-extra/docs/ReleaseNotes.md clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx11.cpp clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp index d687980ed0999..c0ed9849304de 100644 --- a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp @@ -44,6 +44,18 @@ static bool isSingleLine(SourceRange Range, const SourceManager &SM) { SM.getExpansionLineNumber(Range.getEnd()); } +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) && + SM.getExpansionLineNumber(Prev->getLocation()) == + SM.getExpansionLineNumber(Tok.getLocation()); +} + namespace { AST_POLYMORPHIC_MATCHER(isMacro, @@ -110,12 +122,16 @@ 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; + if (isPreprocessorDirectiveToken(*LastTok, *Result.SourceManager, + getLangOpts())) + return; + emitDiag(LastTok->getLocation(), LastTok, DiagKind::Enum, Result, Policy); } diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 3d9e34cc4d44e..5ad0b2d3718b9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -326,6 +326,10 @@ infrastructure are described first, followed by tool-specific sections. 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. + - Fixed a false positive on empty brace initializers of types with default member initializers. 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 5d836b7727082..79cce3c5e68cf 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,6 +55,19 @@ struct PackSingle { PackSingle<int> p1; PackSingle<int, double, char> p3; +// #endif before '}' is not a missing trailing comma. +enum class color_t : unsigned { + RED = 0, + GREEN = 1, + BLUE = 2, + CYAN = 3, +#ifdef USE_MAGENTA + LAST = CYAN, +#else + LAST = BLUE, +#endif +}; + struct WithDefault { int foo = 1; }; void takesTwo(WithDefault, int); 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..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,6 +144,52 @@ void nestedMultiLine() { // CHECK-FIXES-NEXT: }; } +// #endif before '}' is not a missing trailing comma. +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 +}; + +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: }; + +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
