[clang-tools-extra] [clang-tidy] Fix readability-trailing-comma false positive on #endif (PR #219634)
Aayush Mainali via cfe-commits
cfe-commits at lists.llvm.org
Sun Sep 27 22:06:16 PDT 2026
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 <aayushmainali.codes at gmail.com>
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 <aayushmainali.codes at gmail.com>
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 <aayushmainali.codes at gmail.com>
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 <aayushmainali.codes at gmail.com>
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}
More information about the cfe-commits
mailing list