https://github.com/nakasan617 updated https://github.com/llvm/llvm-project/pull/215934
>From 8f5802d940ca31e5464cd84ed9f90c2d2df53106 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 | 3 +- .../readability/trailing-comma-cxx20.cpp | 119 ++++++++++++++++++ .../checkers/readability/trailing-comma.c | 51 ++++++++ 3 files changed, 172 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..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: }; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
