From: John McCall Date: Sat, 10 Apr 2010 09:28:51 +0000 (+0000) Subject: Diagnose more cases of initializing distinct members of an anonymous union X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3c3ccdbe73cb43bdf39a9102f5f7eb842fb71952;p=clang Diagnose more cases of initializing distinct members of an anonymous union member. Use a better diagnostic for this case. Also fix a bug with nested anonymous structs/unions for -Wreorder; this last was PR6575. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@100923 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 859121d6d0..b29d07115f 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2461,10 +2461,11 @@ def err_overload_incorrect_fntype : Error< def err_only_constructors_take_base_inits : Error< "only constructors take base initializers">; -def error_multiple_mem_initialization : Error < - "multiple initializations given for non-static member '%0'">; - -def error_multiple_base_initialization : Error < +def err_multiple_mem_initialization : Error < + "multiple initializations given for non-static member %0">; +def err_multiple_mem_union_initialization : Error < + "initializing multiple members of anonymous union">; +def err_multiple_base_initialization : Error < "multiple initializations given for base %0">; def err_mem_init_not_member_or_class : Error< diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index bc8b7d6bc6..d68351cc38 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -1660,11 +1660,20 @@ static void *GetKeyForMember(ASTContext &Context, if (MemberMaybeAnon && Field->isAnonymousStructOrUnion()) Field = Member->getAnonUnionMember(); - // If the field is a member of an anonymous union, we use record decl of the - // union as the key. + // If the field is a member of an anonymous struct or union, our key + // is the anonymous record decl that's a direct child of the class. RecordDecl *RD = Field->getParent(); - if (RD->isAnonymousStructOrUnion() && RD->isUnion()) + if (RD->isAnonymousStructOrUnion()) { + while (true) { + RecordDecl *Parent = cast(RD->getDeclContext()); + if (Parent->isAnonymousStructOrUnion()) + RD = Parent; + else + break; + } + return static_cast(RD); + } return static_cast(Field); } @@ -1754,6 +1763,71 @@ DiagnoseBaseOrMemInitializerOrder(Sema &SemaRef, } } +namespace { +bool CheckRedundantInit(Sema &S, + CXXBaseOrMemberInitializer *Init, + CXXBaseOrMemberInitializer *&PrevInit) { + if (!PrevInit) { + PrevInit = Init; + return false; + } + + if (FieldDecl *Field = Init->getMember()) + S.Diag(Init->getSourceLocation(), + diag::err_multiple_mem_initialization) + << Field->getDeclName() + << Init->getSourceRange(); + else { + Type *BaseClass = Init->getBaseClass(); + assert(BaseClass && "neither field nor base"); + S.Diag(Init->getSourceLocation(), + diag::err_multiple_base_initialization) + << QualType(BaseClass, 0) + << Init->getSourceRange(); + } + S.Diag(PrevInit->getSourceLocation(), diag::note_previous_initializer) + << 0 << PrevInit->getSourceRange(); + + return true; +} + +typedef std::pair UnionEntry; +typedef llvm::DenseMap RedundantUnionMap; + +bool CheckRedundantUnionInit(Sema &S, + CXXBaseOrMemberInitializer *Init, + RedundantUnionMap &Unions) { + FieldDecl *Field = Init->getMember(); + RecordDecl *Parent = Field->getParent(); + if (!Parent->isAnonymousStructOrUnion()) + return false; + + NamedDecl *Child = Field; + do { + if (Parent->isUnion()) { + UnionEntry &En = Unions[Parent]; + if (En.first && En.first != Child) { + S.Diag(Init->getSourceLocation(), + diag::err_multiple_mem_union_initialization) + << Field->getDeclName() + << Init->getSourceRange(); + S.Diag(En.second->getSourceLocation(), diag::note_previous_initializer) + << 0 << En.second->getSourceRange(); + return true; + } else if (!En.first) { + En.first = Child; + En.second = Init; + } + } + + Child = Parent; + Parent = cast(Parent->getDeclContext()); + } while (Parent->isAnonymousStructOrUnion()); + + return false; +} +} + /// ActOnMemInitializers - Handle the member initializers for a constructor. void Sema::ActOnMemInitializers(DeclPtrTy ConstructorDecl, SourceLocation ColonLoc, @@ -1774,34 +1848,29 @@ void Sema::ActOnMemInitializers(DeclPtrTy ConstructorDecl, CXXBaseOrMemberInitializer **MemInits = reinterpret_cast(meminits); - + + // Mapping for the duplicate initializers check. + // For member initializers, this is keyed with a FieldDecl*. + // For base initializers, this is keyed with a Type*. llvm::DenseMap Members; + + // Mapping for the inconsistent anonymous-union initializers check. + RedundantUnionMap MemberUnions; + bool HadError = false; for (unsigned i = 0; i < NumMemInits; i++) { - CXXBaseOrMemberInitializer *Member = MemInits[i]; + CXXBaseOrMemberInitializer *Init = MemInits[i]; - void *KeyToMember = GetKeyForMember(Context, Member); - CXXBaseOrMemberInitializer *&PrevMember = Members[KeyToMember]; - if (!PrevMember) { - PrevMember = Member; - continue; - } - if (FieldDecl *Field = Member->getMember()) - Diag(Member->getSourceLocation(), - diag::error_multiple_mem_initialization) - << Field->getNameAsString() - << Member->getSourceRange(); - else { - Type *BaseClass = Member->getBaseClass(); - assert(BaseClass && "ActOnMemInitializers - neither field or base"); - Diag(Member->getSourceLocation(), - diag::error_multiple_base_initialization) - << QualType(BaseClass, 0) - << Member->getSourceRange(); + if (Init->isMemberInitializer()) { + FieldDecl *Field = Init->getMember(); + if (CheckRedundantInit(*this, Init, Members[Field]) || + CheckRedundantUnionInit(*this, Init, MemberUnions)) + HadError = true; + } else { + void *Key = GetKeyForBase(Context, QualType(Init->getBaseClass(), 0)); + if (CheckRedundantInit(*this, Init, Members[Key])) + HadError = true; } - Diag(PrevMember->getSourceLocation(), diag::note_previous_initializer) - << 0; - HadError = true; } if (HadError) diff --git a/test/SemaCXX/class-base-member-init.cpp b/test/SemaCXX/class-base-member-init.cpp index 7095d92d68..ca9f045416 100644 --- a/test/SemaCXX/class-base-member-init.cpp +++ b/test/SemaCXX/class-base-member-init.cpp @@ -51,3 +51,26 @@ namespace Test3 { t(2) { } // expected-error {{multiple initializations given for non-static member 't'}} }; } + +namespace test4 { + class A { + union { + struct { + int a; + int b; + }; + + int c; + + union { + int d; + int e; + }; + }; + + A(char _) : a(0), b(0) {} + A(short _) : a(0), c(0) {} // expected-error {{initializing multiple members of anonymous union}} expected-note {{previous initialization is here}} + A(int _) : d(0), e(0) {} // expected-error {{initializing multiple members of anonymous union}} expected-note {{previous initialization is here}} + A(long _) : a(0), d(0) {} // expected-error {{initializing multiple members of anonymous union}} expected-note {{previous initialization is here}} + }; +} diff --git a/test/SemaCXX/constructor-initializer.cpp b/test/SemaCXX/constructor-initializer.cpp index a22c417032..c0e1281375 100644 --- a/test/SemaCXX/constructor-initializer.cpp +++ b/test/SemaCXX/constructor-initializer.cpp @@ -73,8 +73,9 @@ class U { union { int a; char* p; }; union { int b; double d; }; - U() : a(1), p(0), d(1.0) {} // expected-error {{multiple initializations given for non-static member 'p'}} \ - // expected-note {{previous initialization is here}} + U() : a(1), // expected-note {{previous initialization is here}} + p(0), // expected-error {{initializing multiple members of anonymous union}} + d(1.0) {} }; struct V {}; diff --git a/test/SemaCXX/warn-reorder-ctor-initialization.cpp b/test/SemaCXX/warn-reorder-ctor-initialization.cpp index 15e0867b26..3ff01af362 100644 --- a/test/SemaCXX/warn-reorder-ctor-initialization.cpp +++ b/test/SemaCXX/warn-reorder-ctor-initialization.cpp @@ -108,3 +108,15 @@ namespace test2 { Foo y; }; } + +// PR6575: this should not crash +namespace test3 { + struct MyClass { + MyClass() : m_int(0) {} + union { + struct { + int m_int; + }; + }; + }; +}