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] [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} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
