[llvm-branch-commits] [clang-tools-extra] 99b7b38 - [clang-tidy] Fix false positives in readability-trailing-comma for designated initializers (#215934)
Tobias Hieta via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sat Sep 5 00:12:35 PDT 2026
Author: Yuta Nakamura
Date: 2026-09-05T09:12:06+02:00
New Revision: 99b7b38c1b9864da1d66720eb864cf8e6508fd80
URL: https://github.com/llvm/llvm-project/commit/99b7b38c1b9864da1d66720eb864cf8e6508fd80
DIFF: https://github.com/llvm/llvm-project/commit/99b7b38c1b9864da1d66720eb864cf8e6508fd80.diff
LOG: [clang-tidy] Fix false positives in readability-trailing-comma for designated initializers (#215934)
The problem is that we delete the necessary comma whenever we use
implicit initializer lists. How we solve this is that whenever we see an
implicit initializer list, we do not match ``InitListExpr`` nodes at
all, so that we will not delete the necessary comma. Why we chose this
path is detailed in Alternatives considered.
This produces a fix that breaks valid code (#214087) and one that never
converges (#214086).
<details>
<summary><b>Alternatives considered</b></summary>
### 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 in #214087
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 in #214087 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.
</details>
## 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
Fixes: #214087
(cherry picked from commit 6e294bcca832f6e02c0cbc0575393778bd953ce3)
Added:
Modified:
clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma-cxx20.cpp
clang-tools-extra/test/clang-tidy/checkers/readability/trailing-comma.c
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp
index 4dd881cf37993..cb1a33ba09233 100644
--- a/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/TrailingCommaCheck.cpp
@@ -84,7 +84,8 @@ void TrailingCommaCheck::registerMatchers(MatchFinder *Finder) {
.bind("enum"),
this);
- Finder->addMatcher(initListExpr(unless(isEmptyInitList()), unless(isMacro()))
+ 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..db5a3fbeb2a57 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,122 @@ void with_array() {
.count = 3,
};
}
+
+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: };
+
+ 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: };
+}
+
+struct AnonStruct {
+ int x;
+ struct { int p; int q; };
+};
+
+void anonymous_struct_members() {
+ AnonStruct as1 = {
+ .x = 1,
+ .p = 2,
+ .q = 3,
+ };
+
+ AnonStruct as2 = { .x = 1, .p = 2, .q = 3, };
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: initializer list should not have a trailing comma
+ // CHECK-FIXES: AnonStruct as2 = { .x = 1, .p = 2, .q = 3 };
+}
+
+struct Deep { int c; };
+struct Mid { Deep b; };
+struct Top { Mid a; };
+
+void multi_level_designator() {
+ Top t1 = {
+ .a.b.c = 1,
+ };
+
+ Top t2 = {
+ .a.b.c = 1
+ };
+ // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: initializer list should have a trailing comma
+ // CHECK-FIXES: Top t2 = {
+ // CHECK-FIXES-NEXT: .a.b.c = 1,
+ // CHECK-FIXES-NEXT: };
+}
+
+struct TwoFields { int v; int w; };
+struct Holder { TwoFields y; };
+
+void repeated_subobject_designator() {
+ Holder h1 = {
+ .y.v = 1,
+ .y.w = 2,
+ };
+}
+
+struct WithArrayField { int vals[3]; int n; };
+
+void array_designator() {
+ WithArrayField wa1 = {
+ .vals[0] = 1,
+ .n = 1,
+ };
+
+ WithArrayField wa2 = {
+ .vals[0] = 1,
+ .n = 1
+ };
+ // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: initializer list should have a trailing comma
+ // CHECK-FIXES: WithArrayField wa2 = {
+ // CHECK-FIXES-NEXT: .vals[0] = 1,
+ // CHECK-FIXES-NEXT: .n = 1,
+ // 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..9498fead9fcda 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,54 @@ struct Point singleDesig4 = {
// CHECK-FIXES: struct Point singleDesig4 = {
// CHECK-FIXES-NEXT: .x = 10,
// CHECK-FIXES-NEXT: };
+
+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: };
+
+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: };
More information about the llvm-branch-commits
mailing list