From 4dc41c9b766140b507980a13acccf2f05ed19cd3 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Wed, 10 Aug 2011 15:22:55 +0000 Subject: [PATCH] Rewrite default initialization of anonymous structs/unions within a constructor. Previously, we did some bogus recursion into the fields of anonymous structs (recursively), which ended up building invalid ASTs that would cause CodeGen to crash due to invalid GEPs. Now, we instead build the default initializations based on the indirect field declarations at the top level, which properly generates the sequence of GEPs needed to initialize the proper member. Fixes PR10512 and . git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@137212 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaDeclCXX.cpp | 180 +++++++++++------- .../anonymous-union-member-initializer.cpp | 49 +++++ 2 files changed, 160 insertions(+), 69 deletions(-) diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 4671a87423..b5b7a314f0 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -1944,7 +1944,7 @@ BuildImplicitBaseInitializer(Sema &SemaRef, CXXConstructorDecl *Constructor, static bool BuildImplicitMemberInitializer(Sema &SemaRef, CXXConstructorDecl *Constructor, ImplicitInitializerKind ImplicitInitKind, - FieldDecl *Field, + FieldDecl *Field, IndirectFieldDecl *Indirect, CXXCtorInitializer *&CXXMemberInit) { if (Field->isInvalidDecl()) return true; @@ -1968,7 +1968,7 @@ BuildImplicitMemberInitializer(Sema &SemaRef, CXXConstructorDecl *Constructor, CXXScopeSpec SS; LookupResult MemberLookup(SemaRef, Field->getDeclName(), Loc, Sema::LookupMemberName); - MemberLookup.addDecl(Field, AS_public); + MemberLookup.addDecl(Indirect? cast(Indirect) : cast(Field), AS_public); MemberLookup.resolveKind(); ExprResult CopyCtorArg = SemaRef.BuildMemberReferenceExpr(MemberExprBase, @@ -2027,7 +2027,10 @@ BuildImplicitMemberInitializer(Sema &SemaRef, CXXConstructorDecl *Constructor, // of array-subscript entities. SmallVector Entities; Entities.reserve(1 + IndexVariables.size()); - Entities.push_back(InitializedEntity::InitializeMember(Field)); + if (Indirect) + Entities.push_back(InitializedEntity::InitializeMember(Indirect)); + else + Entities.push_back(InitializedEntity::InitializeMember(Field)); for (unsigned I = 0, N = IndexVariables.size(); I != N; ++I) Entities.push_back(InitializedEntity::InitializeElement(SemaRef.Context, 0, @@ -2048,11 +2051,20 @@ BuildImplicitMemberInitializer(Sema &SemaRef, CXXConstructorDecl *Constructor, if (MemberInit.isInvalid()) return true; - CXXMemberInit - = CXXCtorInitializer::Create(SemaRef.Context, Field, Loc, Loc, - MemberInit.takeAs(), Loc, - IndexVariables.data(), - IndexVariables.size()); + if (Indirect) { + assert(IndexVariables.size() == 0 && + "Indirect field improperly initialized"); + CXXMemberInit + = new (SemaRef.Context) CXXCtorInitializer(SemaRef.Context, Indirect, + Loc, Loc, + MemberInit.takeAs(), + Loc); + } else + CXXMemberInit = CXXCtorInitializer::Create(SemaRef.Context, Field, Loc, + Loc, MemberInit.takeAs(), + Loc, + IndexVariables.data(), + IndexVariables.size()); return false; } @@ -2062,7 +2074,9 @@ BuildImplicitMemberInitializer(Sema &SemaRef, CXXConstructorDecl *Constructor, SemaRef.Context.getBaseElementType(Field->getType()); if (FieldBaseElementType->isRecordType()) { - InitializedEntity InitEntity = InitializedEntity::InitializeMember(Field); + InitializedEntity InitEntity + = Indirect? InitializedEntity::InitializeMember(Indirect) + : InitializedEntity::InitializeMember(Field); InitializationKind InitKind = InitializationKind::CreateDefault(Loc); @@ -2074,11 +2088,17 @@ BuildImplicitMemberInitializer(Sema &SemaRef, CXXConstructorDecl *Constructor, if (MemberInit.isInvalid()) return true; - CXXMemberInit = - new (SemaRef.Context) CXXCtorInitializer(SemaRef.Context, - Field, Loc, Loc, - MemberInit.get(), - Loc); + if (Indirect) + CXXMemberInit = new (SemaRef.Context) CXXCtorInitializer(SemaRef.Context, + Indirect, Loc, + Loc, + MemberInit.get(), + Loc); + else + CXXMemberInit = new (SemaRef.Context) CXXCtorInitializer(SemaRef.Context, + Field, Loc, Loc, + MemberInit.get(), + Loc); return false; } @@ -2144,7 +2164,8 @@ struct BaseAndFieldInfo { } static bool CollectFieldInitializer(Sema &SemaRef, BaseAndFieldInfo &Info, - FieldDecl *Top, FieldDecl *Field) { + FieldDecl *Field, + IndirectFieldDecl *Indirect = 0) { // Overwhelmingly common case: we have a direct initializer for this field. if (CXXCtorInitializer *Init = Info.AllBaseFields.lookup(Field)) { @@ -2156,54 +2177,21 @@ static bool CollectFieldInitializer(Sema &SemaRef, BaseAndFieldInfo &Info, // has a brace-or-equal-initializer, the entity is initialized as specified // in [dcl.init]. if (Field->hasInClassInitializer()) { - Info.AllToInit.push_back( - new (SemaRef.Context) CXXCtorInitializer(SemaRef.Context, Field, - SourceLocation(), - SourceLocation(), 0, - SourceLocation())); + CXXCtorInitializer *Init; + if (Indirect) + Init = new (SemaRef.Context) CXXCtorInitializer(SemaRef.Context, Indirect, + SourceLocation(), + SourceLocation(), 0, + SourceLocation()); + else + Init = new (SemaRef.Context) CXXCtorInitializer(SemaRef.Context, Field, + SourceLocation(), + SourceLocation(), 0, + SourceLocation()); + Info.AllToInit.push_back(Init); return false; } - if (Info.IIK == IIK_Default && Field->isAnonymousStructOrUnion()) { - const RecordType *FieldClassType = Field->getType()->getAs(); - assert(FieldClassType && "anonymous struct/union without record type"); - CXXRecordDecl *FieldClassDecl - = cast(FieldClassType->getDecl()); - - // Even though union members never have non-trivial default - // constructions in C++03, we still build member initializers for aggregate - // record types which can be union members, and C++0x allows non-trivial - // default constructors for union members, so we ensure that only one - // member is initialized for these. - if (FieldClassDecl->isUnion()) { - // First check for an explicit initializer for one field. - for (RecordDecl::field_iterator FA = FieldClassDecl->field_begin(), - EA = FieldClassDecl->field_end(); FA != EA; FA++) { - if (CXXCtorInitializer *Init = Info.AllBaseFields.lookup(*FA)) { - Info.AllToInit.push_back(Init); - - // Once we've initialized a field of an anonymous union, the union - // field in the class is also initialized, so exit immediately. - return false; - } else if ((*FA)->isAnonymousStructOrUnion()) { - if (CollectFieldInitializer(SemaRef, Info, Top, *FA)) - return true; - } - } - - // FIXME: C++0x unrestricted unions might call a default constructor here. - return false; - } else { - // For structs, we simply descend through to initialize all members where - // necessary. - for (RecordDecl::field_iterator FA = FieldClassDecl->field_begin(), - EA = FieldClassDecl->field_end(); FA != EA; FA++) { - if (CollectFieldInitializer(SemaRef, Info, Top, *FA)) - return true; - } - } - } - // Don't try to build an implicit initializer if there were semantic // errors in any of the initializers (and therefore we might be // missing some that the user actually wrote). @@ -2211,7 +2199,8 @@ static bool CollectFieldInitializer(Sema &SemaRef, BaseAndFieldInfo &Info, return false; CXXCtorInitializer *Init = 0; - if (BuildImplicitMemberInitializer(Info.S, Info.Ctor, Info.IIK, Field, Init)) + if (BuildImplicitMemberInitializer(Info.S, Info.Ctor, Info.IIK, Field, + Indirect, Init)) return true; if (Init) @@ -2239,7 +2228,20 @@ Sema::SetDelegatingInitializer(CXXConstructorDecl *Constructor, return false; } - + +/// \brief Determine whether the given indirect field declaration is somewhere +/// within an anonymous union. +static bool isWithinAnonymousUnion(IndirectFieldDecl *F) { + for (IndirectFieldDecl::chain_iterator C = F->chain_begin(), + CEnd = F->chain_end(); + C != CEnd; ++C) + if (CXXRecordDecl *Record = dyn_cast((*C)->getDeclContext())) + if (Record->isUnion()) + return true; + + return false; +} + bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, CXXCtorInitializer **Initializers, unsigned NumInitializers, @@ -2331,15 +2333,55 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, } // Fields. - for (CXXRecordDecl::field_iterator Field = ClassDecl->field_begin(), - E = ClassDecl->field_end(); Field != E; ++Field) { - if ((*Field)->getType()->isIncompleteArrayType()) { - assert(ClassDecl->hasFlexibleArrayMember() && - "Incomplete array type is not valid"); + for (DeclContext::decl_iterator Mem = ClassDecl->decls_begin(), + MemEnd = ClassDecl->decls_end(); + Mem != MemEnd; ++Mem) { + if (FieldDecl *F = dyn_cast(*Mem)) { + if (F->getType()->isIncompleteArrayType()) { + assert(ClassDecl->hasFlexibleArrayMember() && + "Incomplete array type is not valid"); + continue; + } + + // If we're not generating the implicit copy constructor, then we'll + // handle anonymous struct/union fields based on their individual + // indirect fields. + if (F->isAnonymousStructOrUnion() && Info.IIK == IIK_Default) + continue; + + if (CollectFieldInitializer(*this, Info, F)) + HadError = true; continue; } - if (CollectFieldInitializer(*this, Info, *Field, *Field)) - HadError = true; + + // Beyond this point, we only consider default initialization. + if (Info.IIK != IIK_Default) + continue; + + if (IndirectFieldDecl *F = dyn_cast(*Mem)) { + if (F->getType()->isIncompleteArrayType()) { + assert(ClassDecl->hasFlexibleArrayMember() && + "Incomplete array type is not valid"); + continue; + } + + // If this field is somewhere within an anonymous union, we only + // initialize it if there's an explicit initializer. + if (isWithinAnonymousUnion(F)) { + if (CXXCtorInitializer *Init + = Info.AllBaseFields.lookup(F->getAnonField())) { + Info.AllToInit.push_back(Init); + } + + continue; + } + + // Initialize each field of an anonymous struct individually. + if (CollectFieldInitializer(*this, Info, F->getAnonField(), F)) + HadError = true; + + continue; + } } NumInitializers = Info.AllToInit.size(); diff --git a/test/CodeGenCXX/anonymous-union-member-initializer.cpp b/test/CodeGenCXX/anonymous-union-member-initializer.cpp index 324ff4aee2..a12ae53f39 100644 --- a/test/CodeGenCXX/anonymous-union-member-initializer.cpp +++ b/test/CodeGenCXX/anonymous-union-member-initializer.cpp @@ -67,6 +67,55 @@ namespace test2 { // CHECK: } } +namespace PR10512 { + struct A { + A(); + A(int); + A(long); + + struct { + struct {int x;}; + struct {int y;}; + }; + }; + + // CHECK: define void @_ZN7PR105121AC2Ev + // CHECK: [[THISADDR:%[a-zA-z0-9.]+]] = alloca [[A:%"struct[A-Za-z0-9:.]+"]] + // CHECK-NEXT: store [[A]]* [[THIS:%[a-zA-z0-9.]+]], [[A]]** [[THISADDR]] + // CHECK-NEXT: [[THIS1:%[a-zA-z0-9.]+]] = load [[A]]** [[THISADDR]] + // CHECK-NEXT: ret void + A::A() {} + + // CHECK: define void @_ZN7PR105121AC2Ei + // CHECK: [[THISADDR:%[a-zA-z0-9.]+]] = alloca [[A:%"struct[A-Za-z0-9:.]+"]] + // CHECK-NEXT: [[XADDR:%[a-zA-z0-9.]+]] = alloca i32 + // CHECK-NEXT: store [[A]]* [[THIS:%[a-zA-z0-9.]+]], [[A]]** [[THISADDR]] + // CHECK-NEXT: store i32 [[X:%[a-zA-z0-9.]+]], i32* [[XADDR]] + // CHECK-NEXT: [[THIS1:%[a-zA-z0-9.]+]] = load [[A]]** [[THISADDR]] + // CHECK-NEXT: {{getelementptr inbounds.*i32 0, i32 0}} + // CHECK-NEXT: {{getelementptr inbounds.*i32 0, i32 0}} + // CHECK-NEXT: {{getelementptr inbounds.*i32 0, i32 0}} + // CHECK-NEXT: [[TMP:%[a-zA-z0-9.]+]] = load i32* [[XADDR]] + // CHECK-NEXT: store i32 [[TMP]] + // CHECK-NEXT: ret void + A::A(int x) : x(x) { } + + // CHECK: define void @_ZN7PR105121AC2El + // CHECK: [[THISADDR:%[a-zA-z0-9.]+]] = alloca [[A:%"struct[A-Za-z0-9:.]+"]] + // CHECK-NEXT: [[XADDR:%[a-zA-z0-9.]+]] = alloca i64 + // CHECK-NEXT: store [[A]]* [[THIS:%[a-zA-z0-9.]+]], [[A]]** [[THISADDR]] + // CHECK-NEXT: store i64 [[X:%[a-zA-z0-9.]+]], i64* [[XADDR]] + // CHECK-NEXT: [[THIS1:%[a-zA-z0-9.]+]] = load [[A]]** [[THISADDR]] + // CHECK-NEXT: {{getelementptr inbounds.*i32 0, i32 0}} + // CHECK-NEXT: {{getelementptr inbounds.*i32 0, i32 1}} + // CHECK-NEXT: {{getelementptr inbounds.*i32 0, i32 0}} + // CHECK-NEXT: [[TMP:%[a-zA-z0-9.]+]] = load i64* [[XADDR]] + // CHECK-NEXT: [[CONV:%[a-zA-z0-9.]+]] = trunc i64 [[TMP]] to i32 + // CHECK-NEXT: store i32 [[CONV]] + // CHECK-NEXT: ret void + A::A(long y) : y(y) { } +} + namespace test3 { struct A { union { -- 2.40.0