From: Richard Smith Date: Sat, 8 Dec 2012 02:01:17 +0000 (+0000) Subject: Implement C++03 [dcl.init]p5's checking for value-initialization of references X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d5bc867f6597ee8d4eb31ea217934e436fc7c7e3;p=clang Implement C++03 [dcl.init]p5's checking for value-initialization of references properly, rather than faking it up by pretending that a reference member makes the default constructor non-trivial. That leads to rejects-valids when putting such types inside unions. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@169662 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h index c13389c62b..4728e500a1 100644 --- a/include/clang/AST/DeclCXX.h +++ b/include/clang/AST/DeclCXX.h @@ -353,6 +353,11 @@ class CXXRecordDecl : public RecordDecl { /// \brief True if any field has an in-class initializer. bool HasInClassInitializer : 1; + /// \brief True if any field is of reference type, and does not have an + /// in-class initializer. In this case, value-initialization of this class + /// is illegal in C++98 even if the class has a trivial default constructor. + bool HasUninitializedReferenceMember : 1; + /// \brief The trivial special members which this class has, per /// C++11 [class.ctor]p5, C++11 [class.copy]p12, C++11 [class.copy]p25, /// C++11 [class.dtor]p5. @@ -982,6 +987,20 @@ public: /// for non-static data members. bool hasInClassInitializer() const { return data().HasInClassInitializer; } + /// \brief Whether this class or any of its subobjects has any members of + /// reference type which would make value-initialization ill-formed, per + /// C++03 [dcl.init]p5: + /// -- if T is a non-union class type without a user-declared constructor, + /// then every non-static data member and base-class component of T is + /// value-initialized + /// [...] + /// A program that calls for [...] value-initialization of an entity of + /// reference type is ill-formed. + bool hasUninitializedReferenceMember() const { + return !isUnion() && !hasUserDeclaredConstructor() && + data().HasUninitializedReferenceMember; + } + /// isPOD - Whether this class is a POD-type (C++ [class]p4), which is a class /// that is an aggregate that has no non-static non-POD data members, no /// reference data members, no user-defined copy assignment operator and no diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index d1066d6219..c21cd794ab 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1238,6 +1238,8 @@ def err_reference_var_requires_init : Error< "declaration of reference variable %0 requires an initializer">; def err_reference_without_init : Error< "reference to type %0 requires an initializer">; +def note_value_initialization_here : Note< + "in value-initialization of type %0 here">; def err_reference_has_multiple_inits : Error< "reference cannot be initialized with multiple values">; def err_init_non_aggr_init_list : Error< diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp index 9f9a1b98ce..35bc993f73 100644 --- a/lib/AST/ASTImporter.cpp +++ b/lib/AST/ASTImporter.cpp @@ -1921,6 +1921,8 @@ bool ASTNodeImporter::ImportDefinition(RecordDecl *From, RecordDecl *To, ToData.HasMutableFields = FromData.HasMutableFields; ToData.HasOnlyCMembers = FromData.HasOnlyCMembers; ToData.HasInClassInitializer = FromData.HasInClassInitializer; + ToData.HasUninitializedReferenceMember + = FromData.HasUninitializedReferenceMember; ToData.HasTrivialSpecialMembers = FromData.HasTrivialSpecialMembers; ToData.HasIrrelevantDestructor = FromData.HasIrrelevantDestructor; ToData.HasConstexprNonCopyMoveConstructor diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp index 210dcb6d33..d3a0c1ad4c 100644 --- a/lib/AST/DeclCXX.cpp +++ b/lib/AST/DeclCXX.cpp @@ -41,7 +41,7 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D) Abstract(false), IsStandardLayout(true), HasNoNonEmptyBases(true), HasPrivateFields(false), HasProtectedFields(false), HasPublicFields(false), HasMutableFields(false), HasOnlyCMembers(true), - HasInClassInitializer(false), + HasInClassInitializer(false), HasUninitializedReferenceMember(false), HasTrivialSpecialMembers(SMF_All), HasIrrelevantDestructor(true), HasConstexprNonCopyMoveConstructor(false), @@ -295,6 +295,9 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases, // Keep track of the presence of mutable fields. if (BaseClassDecl->hasMutableFields()) data().HasMutableFields = true; + + if (BaseClassDecl->hasUninitializedReferenceMember()) + data().HasUninitializedReferenceMember = true; } if (VBases.empty()) @@ -727,7 +730,8 @@ void CXXRecordDecl::addedMember(Decl *D) { data().PlainOldData = false; if (T->isReferenceType()) { - data().HasTrivialSpecialMembers &= ~SMF_DefaultConstructor; + if (!Field->hasInClassInitializer()) + data().HasUninitializedReferenceMember = true; // C++0x [class]p7: // A standard-layout class is a class that: @@ -866,6 +870,10 @@ void CXXRecordDecl::addedMember(Decl *D) { // parameter is of type 'const M&', 'const volatile M&' or 'M'. if (!FieldRec->hasCopyAssignmentWithConstParam()) data().ImplicitCopyAssignmentHasConstParam = false; + + if (FieldRec->hasUninitializedReferenceMember() && + !Field->hasInClassInitializer()) + data().HasUninitializedReferenceMember = true; } } else { // Base element type of field is a non-class type. diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index 48b0d1f896..dcfd18b122 100644 --- a/lib/Sema/SemaInit.cpp +++ b/lib/Sema/SemaInit.cpp @@ -3623,6 +3623,22 @@ static void TryValueInitialization(Sema &S, if (NeedZeroInitialization) Sequence.AddZeroInitializationStep(Entity.getType()); + // C++03: + // -- if T is a non-union class type without a user-declared constructor, + // then every non-static data member and base class component of T is + // value-initialized; + // [...] A program that calls for [...] value-initialization of an + // entity of reference type is ill-formed. + // + // C++11 doesn't need this handling, because value-initialization does not + // occur recursively there, and the implicit default constructor is + // defined as deleted in the problematic cases. + if (!S.getLangOpts().CPlusPlus0x && + ClassDecl->hasUninitializedReferenceMember()) { + Sequence.SetFailed(InitializationSequence::FK_TooManyInitsForReference); + return; + } + // If this is list-value-initialization, pass the empty init list on when // building the constructor call. This affects the semantics of a few // things (such as whether an explicit default constructor can be called). @@ -5443,6 +5459,43 @@ InitializationSequence::Perform(Sema &S, return CurInit; } +/// Somewhere within T there is an uninitialized reference subobject. +/// Dig it out and diagnose it. +bool DiagnoseUninitializedReference(Sema &S, SourceLocation Loc, QualType T) { + if (T->isReferenceType()) { + S.Diag(Loc, diag::err_reference_without_init) + << T.getNonReferenceType(); + return true; + } + + CXXRecordDecl *RD = T->getBaseElementTypeUnsafe()->getAsCXXRecordDecl(); + if (!RD || !RD->hasUninitializedReferenceMember()) + return false; + + for (CXXRecordDecl::field_iterator FI = RD->field_begin(), + FE = RD->field_end(); FI != FE; ++FI) { + if (FI->isUnnamedBitfield()) + continue; + + if (DiagnoseUninitializedReference(S, FI->getLocation(), FI->getType())) { + S.Diag(Loc, diag::note_value_initialization_here) << RD; + return true; + } + } + + for (CXXRecordDecl::base_class_iterator BI = RD->bases_begin(), + BE = RD->bases_end(); + BI != BE; ++BI) { + if (DiagnoseUninitializedReference(S, BI->getLocStart(), BI->getType())) { + S.Diag(Loc, diag::note_value_initialization_here) << RD; + return true; + } + } + + return false; +} + + //===----------------------------------------------------------------------===// // Diagnose initialization failures //===----------------------------------------------------------------------===// @@ -5457,10 +5510,17 @@ bool InitializationSequence::Diagnose(Sema &S, switch (Failure) { case FK_TooManyInitsForReference: // FIXME: Customize for the initialized entity? - if (NumArgs == 0) - S.Diag(Kind.getLocation(), diag::err_reference_without_init) - << DestType.getNonReferenceType(); - else // FIXME: diagnostic below could be better! + if (NumArgs == 0) { + // Dig out the reference subobject which is uninitialized and diagnose it. + // If this is value-initialization, this could be nested some way within + // the target type. + assert(Kind.getKind() == InitializationKind::IK_Value || + DestType->isReferenceType()); + bool Diagnosed = + DiagnoseUninitializedReference(S, Kind.getLocation(), DestType); + assert(Diagnosed && "couldn't find uninitialized reference to diagnose"); + (void)Diagnosed; + } else // FIXME: diagnostic below could be better! S.Diag(Kind.getLocation(), diag::err_reference_has_multiple_inits) << SourceRange(Args[0]->getLocStart(), Args[NumArgs - 1]->getLocEnd()); break; diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index d201f539fa..8c1058e396 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -1099,6 +1099,7 @@ void ASTDeclReader::ReadCXXDefinitionData( Data.HasMutableFields = Record[Idx++]; Data.HasOnlyCMembers = Record[Idx++]; Data.HasInClassInitializer = Record[Idx++]; + Data.HasUninitializedReferenceMember = Record[Idx++]; Data.HasTrivialSpecialMembers = Record[Idx++]; Data.HasIrrelevantDestructor = Record[Idx++]; Data.HasConstexprNonCopyMoveConstructor = Record[Idx++]; diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index b6f798921d..f89ae2ef12 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -4588,6 +4588,7 @@ void ASTWriter::AddCXXDefinitionData(const CXXRecordDecl *D, RecordDataImpl &Rec Record.push_back(Data.HasMutableFields); Record.push_back(Data.HasOnlyCMembers); Record.push_back(Data.HasInClassInitializer); + Record.push_back(Data.HasUninitializedReferenceMember); Record.push_back(Data.HasTrivialSpecialMembers); Record.push_back(Data.HasIrrelevantDestructor); Record.push_back(Data.HasConstexprNonCopyMoveConstructor); diff --git a/test/CXX/class/class.union/p1.cpp b/test/CXX/class/class.union/p1.cpp index ee97410aeb..556a9bba0d 100644 --- a/test/CXX/class/class.union/p1.cpp +++ b/test/CXX/class/class.union/p1.cpp @@ -102,6 +102,12 @@ union U5 { int& i1; // expected-error {{union member 'i1' has reference type 'int &'}} }; +union U6 { + struct S { + int &i; + } s; // ok +}; + template struct Either { bool tag; union { // expected-note 6 {{in instantiation of member class}} diff --git a/test/CXX/dcl.decl/dcl.init/p5.cpp b/test/CXX/dcl.decl/dcl.init/p5.cpp index b50e8d780c..e7ccb2ec41 100644 --- a/test/CXX/dcl.decl/dcl.init/p5.cpp +++ b/test/CXX/dcl.decl/dcl.init/p5.cpp @@ -1,20 +1,48 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s -// FIXME: Very incomplete! - // A program that calls for default-initialization or value-initialization of // an entity of reference type is illformed. If T is a cv-qualified type, the // cv-unqualified version of T is used for these definitions of // zero-initialization, default-initialization, and value-initialization. -// -// FIXME: The diagnostics for these errors are terrible because they fall out -// of the AST representation rather than being explicitly issued during the -// respective initialization forms. -struct S { // expected-error {{implicit default constructor for 'S' must explicitly initialize the reference member}} \ - // expected-note {{candidate constructor (the implicit copy constructor) not viable}} - int& x; // expected-note {{declared here}} + +struct S { // expected-error {{implicit default constructor for 'S' must explicitly initialize the reference member}} + int &x; // expected-note {{declared here}} expected-error 3{{reference to type 'int' requires an initializer}} }; S s; // expected-note {{implicit default constructor for 'S' first required here}} S f() { - return S(); // expected-error {{no matching constructor for initialization of 'S'}} + return S(); // expected-note {{in value-initialization of type 'S' here}} } + +struct T + : S { // expected-note 2{{in value-initialization of type 'S' here}} +}; +T t = T(); // expected-note {{in value-initialization of type 'T' here}} + +struct U { + T t[3]; // expected-note {{in value-initialization of type 'T' here}} +}; +U u = U(); // expected-note {{in value-initialization of type 'U' here}} + +// Ensure that we handle C++11 in-class initializers properly as an extension. +// In this case, there is no user-declared default constructor, so we +// recursively apply the value-initialization checks, but we will emit a +// constructor call anyway, because the default constructor is not trivial. +struct V { + int n; + int &r = n; // expected-warning {{C++11}} +}; +V v = V(); // ok +struct W { + int n; + S s = { n }; // expected-warning {{C++11}} +}; +W w = W(); // ok + +// Ensure we're not faking this up by making the default constructor +// non-trivial. +#define static_assert(B, S) typedef int assert_failed[(B) ? 1 : -1]; +static_assert(__has_trivial_constructor(S), ""); +static_assert(__has_trivial_constructor(T), ""); +static_assert(__has_trivial_constructor(U), ""); +static_assert(!__has_trivial_constructor(V), ""); +static_assert(!__has_trivial_constructor(W), "");