https://github.com/nakasan617 updated https://github.com/llvm/llvm-project/pull/215934
>From 29151ce48b65e65579d2a18df7e14974a8c79ab3 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 | 56 +++++++++++++++++++ .../checkers/readability/trailing-comma.c | 52 +++++++++++++++++ 3 files changed, 110 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..c45548b5f9eea 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,59 @@ 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: }; +} 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..961394e268feb 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,55 @@ 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
