https://github.com/keinflue updated https://github.com/llvm/llvm-project/pull/153140
>From cfca176239f48de13897b6d1a59eab74b55055af Mon Sep 17 00:00:00 2001 From: keinflue <keinf...@posteo.de> Date: Mon, 11 Aug 2025 15:51:14 +0200 Subject: [PATCH 1/3] [clang] Inject IndirectFieldDecl even if name conflicts. This modifes InjectAnonymousStructOrUnionMembers to inject an IndirectFieldDecl and mark it invalid even if its name conflicts with another name in the scope. This resolves a crash on a further diagnostic diag::err_multople_mem_union_initialization which via findDefaultInitializer relies on these declarations being present. Fixes #149985 --- clang/docs/ReleaseNotes.rst | 2 + clang/lib/Sema/SemaDecl.cpp | 60 ++++++++++--------- clang/test/CXX/class/class.mem/p13.cpp | 2 +- .../class/class.union/class.union.anon/p4.cpp | 10 ++++ 4 files changed, 45 insertions(+), 29 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 0e9fcaa5fac6a..00b21471f3731 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -184,6 +184,8 @@ Bug Fixes to C++ Support (``[[assume(expr)]]``) creates temporary objects. - Fix the dynamic_cast to final class optimization to correctly handle casts that are guaranteed to fail (#GH137518). +- Fix a crash if errors "member of anonymous [...] redeclares" and + "intializing multiple members of union" coincide (#GH149985). Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index b5eb825eb52cc..052e026fa3ee3 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -5463,40 +5463,44 @@ InjectAnonymousStructOrUnionMembers(Sema &SemaRef, Scope *S, DeclContext *Owner, // distinct from the names of any other entity in the // scope in which the anonymous union is declared. Invalid = true; - } else { - // C++ [class.union]p2: - // For the purpose of name lookup, after the anonymous union - // definition, the members of the anonymous union are - // considered to have been defined in the scope in which the - // anonymous union is declared. - unsigned OldChainingSize = Chaining.size(); - if (IndirectFieldDecl *IF = dyn_cast<IndirectFieldDecl>(VD)) - Chaining.append(IF->chain_begin(), IF->chain_end()); - else - Chaining.push_back(VD); + } + // Inject the IndirectFieldDecl even if invalid, because later + // diagnostics may depend on it being present. + + // C++ [class.union]p2: + // For the purpose of name lookup, after the anonymous union + // definition, the members of the anonymous union are + // considered to have been defined in the scope in which the + // anonymous union is declared. + unsigned OldChainingSize = Chaining.size(); + if (IndirectFieldDecl *IF = dyn_cast<IndirectFieldDecl>(VD)) + Chaining.append(IF->chain_begin(), IF->chain_end()); + else + Chaining.push_back(VD); - assert(Chaining.size() >= 2); - NamedDecl **NamedChain = - new (SemaRef.Context)NamedDecl*[Chaining.size()]; - for (unsigned i = 0; i < Chaining.size(); i++) - NamedChain[i] = Chaining[i]; + assert(Chaining.size() >= 2); + NamedDecl **NamedChain = + new (SemaRef.Context) NamedDecl *[Chaining.size()]; + for (unsigned i = 0; i < Chaining.size(); i++) + NamedChain[i] = Chaining[i]; - IndirectFieldDecl *IndirectField = IndirectFieldDecl::Create( - SemaRef.Context, Owner, VD->getLocation(), VD->getIdentifier(), - VD->getType(), {NamedChain, Chaining.size()}); + IndirectFieldDecl *IndirectField = IndirectFieldDecl::Create( + SemaRef.Context, Owner, VD->getLocation(), VD->getIdentifier(), + VD->getType(), {NamedChain, Chaining.size()}); - for (const auto *Attr : VD->attrs()) - IndirectField->addAttr(Attr->clone(SemaRef.Context)); + for (const auto *Attr : VD->attrs()) + IndirectField->addAttr(Attr->clone(SemaRef.Context)); - IndirectField->setAccess(AS); - IndirectField->setImplicit(); - SemaRef.PushOnScopeChains(IndirectField, S); + IndirectField->setAccess(AS); + IndirectField->setImplicit(); + IndirectField->setInvalidDecl(Invalid); + SemaRef.PushOnScopeChains(IndirectField, S); - // That includes picking up the appropriate access specifier. - if (AS != AS_none) IndirectField->setAccess(AS); + // That includes picking up the appropriate access specifier. + if (AS != AS_none) + IndirectField->setAccess(AS); - Chaining.resize(OldChainingSize); - } + Chaining.resize(OldChainingSize); } } diff --git a/clang/test/CXX/class/class.mem/p13.cpp b/clang/test/CXX/class/class.mem/p13.cpp index d947586c41940..4dc959cd0beb8 100644 --- a/clang/test/CXX/class/class.mem/p13.cpp +++ b/clang/test/CXX/class/class.mem/p13.cpp @@ -63,7 +63,7 @@ struct X4 { // expected-note{{previous}} int X; union { float Y; - unsigned X4; // expected-error{{redeclares 'X4'}} + unsigned X4; // expected-error{{redeclares 'X4'}} expected-error {{'X4' has the same name as its class}} }; }; }; diff --git a/clang/test/CXX/class/class.union/class.union.anon/p4.cpp b/clang/test/CXX/class/class.union/class.union.anon/p4.cpp index a12ec38503fa8..710636d2235db 100644 --- a/clang/test/CXX/class/class.union/class.union.anon/p4.cpp +++ b/clang/test/CXX/class/class.union/class.union.anon/p4.cpp @@ -8,3 +8,13 @@ union U { int y = 1; // expected-error {{initializing multiple members of union}} }; }; + +namespace GH149985 { + union U { + int x; // expected-note {{previous declaration is here}} + union { + int x = {}; // expected-error {{member of anonymous union redeclares}} expected-note {{previous initialization is here}} + }; + int y = {}; // expected-error {{initializing multiple members of union}} + }; +} >From c1a5c2cc50dfa5955f9be53bba5cf7a45d56d154 Mon Sep 17 00:00:00 2001 From: keinflue <keinf...@posteo.de> Date: Thu, 14 Aug 2025 14:44:52 +0200 Subject: [PATCH 2/3] Implement review suggestions Disable duplicate diagnostic, include original test case and fix setting only affected fields invalid. --- clang/lib/Sema/SemaDecl.cpp | 23 +++++++++++-------- clang/lib/Sema/SemaDeclCXX.cpp | 5 +++- clang/test/CXX/class/class.mem/p13.cpp | 2 +- .../class/class.union/class.union.anon/p4.cpp | 15 ++++++++++++ 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 008a25fe739e3..343d4c78e401c 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -5494,6 +5494,7 @@ InjectAnonymousStructOrUnionMembers(Sema &SemaRef, Scope *S, DeclContext *Owner, RecordDecl *AnonRecord, AccessSpecifier AS, StorageClass SC, SmallVectorImpl<NamedDecl *> &Chaining) { + bool Invalid = false; // Look every FieldDecl and IndirectFieldDecl with a name. @@ -5501,17 +5502,19 @@ InjectAnonymousStructOrUnionMembers(Sema &SemaRef, Scope *S, DeclContext *Owner, if ((isa<FieldDecl>(D) || isa<IndirectFieldDecl>(D)) && cast<NamedDecl>(D)->getDeclName()) { ValueDecl *VD = cast<ValueDecl>(D); - if (CheckAnonMemberRedeclaration(SemaRef, S, Owner, VD->getDeclName(), - VD->getLocation(), AnonRecord->isUnion(), - SC)) { - // C++ [class.union]p2: - // The names of the members of an anonymous union shall be - // distinct from the names of any other entity in the - // scope in which the anonymous union is declared. + // C++ [class.union]p2: + // The names of the members of an anonymous union shall be + // distinct from the names of any other entity in the + // scope in which the anonymous union is declared. + + const bool FieldInvalid = CheckAnonMemberRedeclaration( + SemaRef, S, Owner, VD->getDeclName(), VD->getLocation(), + AnonRecord->isUnion(), SC); + if (FieldInvalid) Invalid = true; - } + // Inject the IndirectFieldDecl even if invalid, because later - // diagnostics may depend on it being present. + // diagnostics may depend on it being present, see findDefaultInitializer. // C++ [class.union]p2: // For the purpose of name lookup, after the anonymous union @@ -5539,7 +5542,7 @@ InjectAnonymousStructOrUnionMembers(Sema &SemaRef, Scope *S, DeclContext *Owner, IndirectField->setAccess(AS); IndirectField->setImplicit(); - IndirectField->setInvalidDecl(Invalid); + IndirectField->setInvalidDecl(FieldInvalid); SemaRef.PushOnScopeChains(IndirectField, S); // That includes picking up the appropriate access specifier. diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 0477d37cac4c5..dd66a5f15a970 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -7012,9 +7012,12 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl *Record) { for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) { NamedDecl *D = (*I)->getUnderlyingDecl(); + // Invalid IndirectFieldDecls have already been diagnosed with + // err_anonymous_record_member_redecl in + // SemaDecl.cpp:CheckAnonMemberRedeclaration. if (((isa<FieldDecl>(D) || isa<UnresolvedUsingValueDecl>(D)) && Record->hasUserDeclaredConstructor()) || - isa<IndirectFieldDecl>(D)) { + (isa<IndirectFieldDecl>(D) && !D->isInvalidDecl())) { Diag((*I)->getLocation(), diag::err_member_name_of_class) << D->getDeclName(); break; diff --git a/clang/test/CXX/class/class.mem/p13.cpp b/clang/test/CXX/class/class.mem/p13.cpp index 4dc959cd0beb8..d947586c41940 100644 --- a/clang/test/CXX/class/class.mem/p13.cpp +++ b/clang/test/CXX/class/class.mem/p13.cpp @@ -63,7 +63,7 @@ struct X4 { // expected-note{{previous}} int X; union { float Y; - unsigned X4; // expected-error{{redeclares 'X4'}} expected-error {{'X4' has the same name as its class}} + unsigned X4; // expected-error{{redeclares 'X4'}} }; }; }; diff --git a/clang/test/CXX/class/class.union/class.union.anon/p4.cpp b/clang/test/CXX/class/class.union/class.union.anon/p4.cpp index 710636d2235db..f124ac1052b7e 100644 --- a/clang/test/CXX/class/class.union/class.union.anon/p4.cpp +++ b/clang/test/CXX/class/class.union/class.union.anon/p4.cpp @@ -10,6 +10,21 @@ union U { }; namespace GH149985 { + union X { + enum { + csize = 42, + cs = sizeof(int) // expected-note {{previous declaration is here}} + }; + struct { + int data; // expected-note {{previous declaration is here}} + union X *cs[csize] = {}; // expected-error {{member of anonymous struct redeclares}} expected-note {{previous initialization is here}} + }; + struct { + int data; // expected-error {{member of anonymous struct redeclares}} + union X *ds[2] = {}; // expected-error {{initializing multiple members of union}} + }; + }; + union U { int x; // expected-note {{previous declaration is here}} union { >From 16f7cb8100b39b80a245748adc31248f896f2005 Mon Sep 17 00:00:00 2001 From: keinflue <keinf...@posteo.de> Date: Thu, 14 Aug 2025 14:58:13 +0200 Subject: [PATCH 3/3] Extend release notes --- clang/docs/ReleaseNotes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 9bb25fc182fa7..118d7d7838c25 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -74,6 +74,7 @@ AST Dumping Potentially Breaking Changes Clang Frontend Potentially Breaking Changes ------------------------------------------- +- Members of anonymous unions/structs are now injected as ``IndirectFieldDecl`` into the enclosing record even if their names conflict with other names in the scope. These ``IndirectFieldDecl`` are marked invalid. Clang Python Bindings Potentially Breaking Changes -------------------------------------------------- _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits