https://github.com/nakasan617 created
https://github.com/llvm/llvm-project/pull/215934
For a designated initializer, Sema synthesizes an `InitListExpr` for each
intermediate subobject named by the designator. Those nodes have no braces in
the source, and their brace locations point at wherever the designator happens
to begin and end.
`readability-trailing-comma` matched every `InitListExpr`, so a synthesized node
would measure itself as single-line, select `SingleLineCommaPolicy`, then lex
past its non-existent closing brace and rewrite the trailing comma belonging to
the enclosing written list.
This produced two distinct failures.
**#214087 — the fix breaks valid code.** The anonymous union and anonymous
struct each get a synthesized node, and each deletes one comma:
```c++
struct S {
int x;
union { struct { int a; int b; }; };
};
S make(int a, int b) {
return S{
.x = 1,
.a = a, // comma deleted by the synthesized union node
.b = b, // comma deleted by the synthesized struct node
};
}
```
After `--fix` this reads `.a = a.b = b` — member access on an `int`, which no
longer compiles.
**#214086 — the fix never converges.** The synthesized node for `y` is
single-line (`Remove`) while the real enclosing list is multi-line (`Append`),
so the two disagree about the same comma:
```c++
struct A { int v; };
struct B { A x; A y; };
B make(int v) {
return B{
.x = {.v = 1},
.y.v = v, // synthesized node removes it; outer list appends it back
};
}
```
## Fix
A trailing comma belongs to a brace pair, so only lists actually written with
braces can have one. That is exactly what `InitListExpr::isExplicit()` records:
the parser sets it when it consumes braces (`Sema::ActOnInitList`), and Sema
clears it for synthesized subobject lists
(`InitListChecker::createInitListExpr`). The check now skips implicit lists:
```c++
Finder->addMatcher(initListExpr(unless(isEmptyInitList()), unless(isMacro()),
unless(isImplicit()))
.bind("initlist"),
this);
```
This is the same kind of rule as the two guards already present: each says "this
node has no trailing comma we can reason about, so do not look at it".
### Why not repair the source ranges instead
The synthesized nodes also carry misleading locations - their range is a
snapshot of the designator that caused them to be created, so it need not cover
their own children. The anonymous-struct node above reports `9:5-9:10` while
holding an initializer on line 10, which is why it measures as single-line.
Repairing that would be a change to Clang rather than to the check, and it is
not clear there is anything to repair: the semantic form exists to record which
initializer belongs to which subobject, and nothing in Clang relies on its
ranges bracketing their children. Only a source-rewriting tool needs that
guarantee, and such a tool should not be inspecting nodes that were never
written. Skipping them is both smaller and better scoped.
### Why not match on brace locations
`getLBraceLoc()`/`getRBraceLoc()` are not a reliable discriminator. On the
synthesized anonymous-struct node above both are *valid*, pointing at the `.`
and at `a`. A validity check would suppress only one of the two false positives
in #214087 and none of #214086. This was in fact the previous implementation of
`isExplicit()`, replaced in #195175 for the same reason.
### Note on scope
No diagnostic is lost. The comma these nodes were rewriting belongs to the
enclosing written list, which is matched independently and still appends or
removes it as its own policy requires; the tests below cover that directly.
## Tests
Both shapes reproduce in C as well as C++ - designated initializers are a C
feature, and anonymous members are C11 - so tests are added to both
`trailing-comma.c` and `trailing-comma-cxx20.cpp`. Before the change the C
reproducer emits 7 diagnostics, 4 of them false positives; after it emits 3,
all correct.
Each file covers:
- the anonymous-union shape and the nested-designator shape, both of which must
now be silent when the enclosing list already has its trailing comma
- the same two shapes with the trailing comma missing, which must still be
diagnosed on the enclosing list
- a nested list written *with* braces, which must still own its own trailing
comma, to confirm the change does not over-suppress
The existing `readability-trailing-comma` tests are unchanged and still pass.
## AI disclosure
What Claude did
- Explored the codebase to locate the check and the relevant Sema/AST machinery
- Instrumented the check with temporary debug output and measured the actual
InitListExpr properties on the reproducers (brace locations, isExplicit,
computed policies)
- Wrote the final two-line patch and all the added test cases
- Found the isExplicit() history (PR #195175) that the fix depends on
What I did
- Directed the approach and interrogated the reasoning at each step
- Reviewed and verified the results locally
Fixes: #214086, #214087
>From cb0a30726aabfd1edf67b26a02e27db6406e3f53 Mon Sep 17 00:00:00 2001
From: Yuta Nakamura <[email protected]>
Date: Wed, 12 Aug 2026 08:37:32 -0500
Subject: [PATCH] [clang-tidy] Fix false positives in
readability-trailing-comma for designated initializers
For a designated initializer, Sema synthesizes an InitListExpr for each
intermediate subobject named by the designator. Those nodes have no braces
in the source, and their brace locations point at wherever the designator
happens to begin and end.
The check matched every InitListExpr, so a synthesized node would measure
itself as single-line, select SingleLineCommaPolicy, then lex past its
non-existent closing brace and rewrite the trailing comma belonging to the
enclosing written list.
This produced two failures:
S{.x = 1, .a = a, .b = b,} the anonymous union and struct nodes each
deleted one comma, yielding '.a = a.b = b'
B{.y.v = v,} the synthesized node deleted the comma while
the multi-line outer list re-appended it, so
--fix never converged
A trailing comma belongs to a brace pair, so only lists actually written
with braces can have one. InitListExpr::isExplicit() records exactly that:
the parser sets it when it consumes braces, and Sema clears it for
synthesized subobject lists. Skip implicit lists in the matcher.
Both shapes reproduce in C as well as C++, so tests are added to both
trailing-comma.c and trailing-comma-cxx20.cpp, covering the false positives,
the enclosing list still being diagnosed when its comma is absent, and a
nested list written with braces still owning its own comma.
Fixes: #214086, #214087
---
.../readability/TrailingCommaCheck.cpp | 9 ++-
.../readability/trailing-comma-cxx20.cpp | 61 +++++++++++++++++++
.../checkers/readability/trailing-comma.c | 57 +++++++++++++++++
3 files changed, 126 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp
b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp
index 4dd881cf37993..4a431de5c6a32 100644
--- a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp
@@ -84,7 +84,14 @@ void TrailingCommaCheck::registerMatchers(MatchFinder
*Finder) {
.bind("enum"),
this);
- Finder->addMatcher(initListExpr(unless(isEmptyInitList()), unless(isMacro()))
+ // Sema synthesizes InitListExpr nodes for the subobjects addressed by a
+ // designated initializer, e.g. the anonymous union and struct in
+ // 'S{.x = 1, .a = a}' or the 'y' subobject in 'B{.y.v = v}'. Those nodes
have
+ // no braces in the source, and their brace locations point at whatever the
+ // designator happens to begin and end with. A trailing comma belongs to a
+ // brace pair, so only lists actually written with braces can have one.
+ Finder->addMatcher(initListExpr(unless(isEmptyInitList()), unless(isMacro()),
+ unless(isImplicit()))
.bind("initlist"),
this);
}
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx20.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx20.cpp
index b2f6ed072563f..b4b9485dacb10 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx20.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx20.cpp
@@ -87,3 +87,64 @@ void with_array() {
.count = 3,
};
}
+
+// Sema synthesizes InitListExpr nodes for the subobjects named by a designated
+// initializer. Those nodes have no braces in the source, so the trailing comma
+// they see belongs to the enclosing written list and must not be rewritten.
+
+struct AnonUnion {
+ int x;
+ union { struct { int a; int b; }; };
+};
+
+void anonymous_union_members() {
+ AnonUnion w1 = {
+ .x = 1,
+ .a = 2,
+ .b = 3,
+ };
+
+ AnonUnion w2 = {
+ .x = 1,
+ .a = 2,
+ .b = 3
+ };
+ // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: initializer list should have a
trailing comma
+ // CHECK-FIXES: AnonUnion w2 = {
+ // CHECK-FIXES-NEXT: .x = 1,
+ // CHECK-FIXES-NEXT: .a = 2,
+ // CHECK-FIXES-NEXT: .b = 3,
+ // CHECK-FIXES-NEXT: };
+}
+
+struct Inner { int v; };
+struct Nested { Inner x; Inner y; };
+
+void nested_designator() {
+ Nested n1 = {
+ .x = {.v = 1},
+ .y.v = 2,
+ };
+
+ Nested n2 = {
+ .x = {.v = 1},
+ .y.v = 2
+ };
+ // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: initializer list should have a
trailing comma
+ // CHECK-FIXES: Nested n2 = {
+ // CHECK-FIXES-NEXT: .x = {.v = 1},
+ // CHECK-FIXES-NEXT: .y.v = 2,
+ // CHECK-FIXES-NEXT: };
+
+ // A subobject list that *was* written with braces still owns its own
+ // trailing comma, independently of the enclosing list.
+ Nested n3 = {
+ .x = {.v = 1},
+ .y = {.v = 2,},
+ };
+ // CHECK-MESSAGES: :[[@LINE-2]]:17: warning: initializer list should not
have a trailing comma
+ // CHECK-FIXES: Nested n3 = {
+ // CHECK-FIXES-NEXT: .x = {.v = 1},
+ // CHECK-FIXES-NEXT: .y = {.v = 2},
+ // CHECK-FIXES-NEXT: };
+}
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.c
b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.c
index bdf9912e54155..35bb0c59c6a88 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.c
@@ -115,3 +115,60 @@ struct Point singleDesig4 = {
// CHECK-FIXES: struct Point singleDesig4 = {
// CHECK-FIXES-NEXT: .x = 10,
// CHECK-FIXES-NEXT: };
+
+// Sema synthesizes InitListExpr nodes for the subobjects named by a designated
+// initializer. Those nodes have no braces in the source, so the trailing comma
+// they see belongs to the enclosing written list and must not be rewritten.
+
+struct AnonUnion {
+ int x;
+ union { struct { int a; int b; }; };
+};
+
+struct AnonUnion au1 = {
+ .x = 1,
+ .a = 2,
+ .b = 3,
+};
+
+struct AnonUnion au2 = {
+ .x = 1,
+ .a = 2,
+ .b = 3
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: initializer list should have a
trailing comma
+// CHECK-FIXES: struct AnonUnion au2 = {
+// CHECK-FIXES-NEXT: .x = 1,
+// CHECK-FIXES-NEXT: .a = 2,
+// CHECK-FIXES-NEXT: .b = 3,
+// CHECK-FIXES-NEXT: };
+
+struct Inner { int v; };
+struct Outer { struct Inner x; struct Inner y; };
+
+struct Outer nd1 = {
+ .x = {.v = 1},
+ .y.v = 2,
+};
+
+struct Outer nd2 = {
+ .x = {.v = 1},
+ .y.v = 2
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:11: warning: initializer list should have a
trailing comma
+// CHECK-FIXES: struct Outer nd2 = {
+// CHECK-FIXES-NEXT: .x = {.v = 1},
+// CHECK-FIXES-NEXT: .y.v = 2,
+// CHECK-FIXES-NEXT: };
+
+// A subobject list that *was* written with braces still owns its own trailing
+// comma, independently of the enclosing list.
+struct Outer nd3 = {
+ .x = {.v = 1},
+ .y = {.v = 2,},
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:15: warning: initializer list should not have
a trailing comma
+// CHECK-FIXES: struct Outer nd3 = {
+// CHECK-FIXES-NEXT: .x = {.v = 1},
+// CHECK-FIXES-NEXT: .y = {.v = 2},
+// CHECK-FIXES-NEXT: };
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
