rnk created this revision.
rnk added a reviewer: rsmith.
rnk added a subscriber: cfe-commits.
This both improves diagnostic quality and fixes crashes on invalid when
we sneak dependent types into a constructor that isn't in a dependent
context.
Fixes PR24658
http://reviews.llvm.org/D15436
Files:
lib/Sema/SemaDeclCXX.cpp
test/SemaTemplate/class-template-ctor-initializer.cpp
test/SemaTemplate/default-expr-arguments.cpp
test/SemaTemplate/instantiate-member-initializers.cpp
Index: test/SemaTemplate/instantiate-member-initializers.cpp
===================================================================
--- test/SemaTemplate/instantiate-member-initializers.cpp
+++ test/SemaTemplate/instantiate-member-initializers.cpp
@@ -17,7 +17,42 @@
int b;
};
-B<int> b0; // expected-note {{in instantiation of member function 'B<int>::B' requested here}}
+B<int> b0;
+
+namespace test2 {
+struct A {};
+struct B {};
+template <typename T>
+struct C : A, B {
+ int f1;
+ int f2;
+ C() : A()
+ , f1(0) // expected-warning {{field 'f1' will be initialized after base 'T'}}
+ , T()
+ , f2(0)
+ {}
+};
+C<B> c;
+
+template <typename T>
+struct D : A, B {
+ int f1;
+ int f2;
+ D() : A()
+ , T()
+ , f2(0) // expected-warning {{field 'f2' will be initialized after field 'f1'}}
+ , f1(0)
+ {}
+};
+D<B> d;
+
+template <typename T>
+struct E : A, B {
+ // expected-warning@+1 {{base class 'test2::B' will be initialized after base 'test2::A'}}
+ E() : T(), A() {}
+};
+E<B> e; // expected-note-re {{in instantiation {{.*}} requested here}}
+}
template <class T> struct AA { AA(int); };
template <class T> class BB : public AA<T> {
Index: test/SemaTemplate/default-expr-arguments.cpp
===================================================================
--- test/SemaTemplate/default-expr-arguments.cpp
+++ test/SemaTemplate/default-expr-arguments.cpp
@@ -158,13 +158,18 @@
// PR5810
namespace PR5810 {
- template<typename T>
- struct allocator {
- allocator() { int a[sizeof(T) ? -1 : -1]; } // expected-error2 {{array with a negative size}}
- };
+template <typename T>
+struct allocator {
+ // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+#if __cplusplus >= 201103L
+ // expected-note@-3 {{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
+#endif
+ allocator() { int a[sizeof(T) ? -1 : -1]; } // expected-error2 {{array with a negative size}}
+};
template<typename T>
struct vector {
+ // expected-error@+1 {{no matching constructor for initialization of 'allocator<PR5810::B>'}}
vector(const allocator<T>& = allocator<T>()) {} // expected-note2 {{instantiation of}}
};
@@ -183,6 +188,7 @@
template<typename T>
struct X {
vector<B> bs;
+ // expected-note@+1 {{in instantiation of default function argument expression for 'vector<PR5810::B>' required here}}
X() { }
};
Index: test/SemaTemplate/class-template-ctor-initializer.cpp
===================================================================
--- test/SemaTemplate/class-template-ctor-initializer.cpp
+++ test/SemaTemplate/class-template-ctor-initializer.cpp
@@ -57,22 +57,53 @@
}
namespace NonDependentError {
+ // expected-note@+1 2 {{declared here}}
struct Base { Base(int); }; // expected-note {{candidate constructor not viable}}
// expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable}}
#if __cplusplus >= 201103L // C++11 or later
// expected-note@-3 {{candidate constructor (the implicit move constructor) not viable}}
#endif
template<typename T>
struct Derived1 : Base {
+ // expected-error@+1 {{must explicitly initialize the base class}}
Derived1() : Base(1, 2) {} // expected-error {{no matching constructor}}
};
template<typename T>
struct Derived2 : Base {
+ // expected-error@+1 {{must explicitly initialize the base class}}
Derived2() : BaseClass(1) {} // expected-error {{does not name a non-static data member or base}}
};
- Derived1<void> d1;
- Derived2<void> d2;
+ Derived1<void> d1; // expected-note {{requested here}}
+ Derived2<void> d2; // expected-note {{requested here}}
+}
+
+namespace pr24658 {
+// expected-error@+2 {{cannot be defined in a type specifier}}
+// expected-error@+1 {{non-type template parameter cannot}}
+template <typename T, class A : T{A(){}}> void f();
+
+#if __cplusplus >= 201402L
+// expected-error@+1 {{cannot be defined in a type specifier}}
+template <typename T> int x = sizeof(struct B : T{B(){}});
+#endif
+
+// expected-note@+1 {{'pr24658::C' declared here}}
+struct C { C(int); };
+
+template <typename T> struct D {
+ // expected-note@+1 {{member is declared here}}
+ C c;
+ // expected-error@+1 {{must explicitly initialize the member 'c'}}
+ D() {}
+};
+
+struct X { };
+
+template <typename T> struct E : X, C {
+ E() : X(), T(0) {}
+};
+E<C> y;
}
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -3823,6 +3823,10 @@
if (Info.AnyErrorsInInits)
return false;
+ // Don't try to implicitly initialize any fields with dependent types.
+ if (Field->getType()->isDependentType())
+ return false;
+
CXXCtorInitializer *Init = nullptr;
if (BuildImplicitMemberInitializer(Info.S, Info.Ctor, Info.IIK, Field,
Indirect, Init))
@@ -3856,43 +3860,42 @@
return false;
}
+static bool
+hasDependentBasesOrInitializers(const CXXRecordDecl *RD,
+ ArrayRef<CXXCtorInitializer *> Inits) {
+ for (const CXXBaseSpecifier &Base : RD->bases())
+ if (Base.getType()->isDependentType())
+ return true;
+ for (CXXCtorInitializer *Init : Inits)
+ if (Init->isBaseInitializer() && Init->getBaseClass()->isDependentType())
+ return true;
+ return false;
+}
+
bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors,
ArrayRef<CXXCtorInitializer *> Initializers) {
- if (Constructor->isDependentContext()) {
- // Just store the initializers as written, they will be checked during
- // instantiation.
- if (!Initializers.empty()) {
- Constructor->setNumCtorInitializers(Initializers.size());
- CXXCtorInitializer **baseOrMemberInitializers =
- new (Context) CXXCtorInitializer*[Initializers.size()];
- memcpy(baseOrMemberInitializers, Initializers.data(),
- Initializers.size() * sizeof(CXXCtorInitializer*));
- Constructor->setCtorInitializers(baseOrMemberInitializers);
- }
-
- // Let template instantiation know whether we had errors.
- if (AnyErrors)
- Constructor->setInvalidDecl();
-
- return false;
- }
-
BaseAndFieldInfo Info(*this, Constructor, AnyErrors);
// We need to build the initializer AST according to order of construction
// and not what user specified in the Initializers list.
CXXRecordDecl *ClassDecl = Constructor->getParent()->getDefinition();
if (!ClassDecl)
return true;
-
- bool HadError = false;
- for (unsigned i = 0; i < Initializers.size(); i++) {
- CXXCtorInitializer *Member = Initializers[i];
+ // If there are any dependent bases or dependent initializers, we won't be
+ // able to establish the correct ordering until after instantiation. For now,
+ // we move bases to the front of the list, and leave them in source order.
+ bool HasDependentBasesOrInits =
+ hasDependentBasesOrInitializers(ClassDecl, Initializers);
+ bool HadError = false;
- if (Member->isBaseInitializer())
- Info.AllBaseFields[Member->getBaseClass()->getAs<RecordType>()] = Member;
- else {
+ for (CXXCtorInitializer *Member : Initializers) {
+ if (Member->isBaseInitializer()) {
+ if (!HasDependentBasesOrInits) {
+ Info.AllBaseFields[Member->getBaseClass()->castAs<RecordType>()] =
+ Member;
+ }
+ } else {
Info.AllBaseFields[Member->getAnyMember()->getCanonicalDecl()] = Member;
if (IndirectFieldDecl *F = Member->getIndirectMember()) {
@@ -3910,67 +3913,69 @@
}
}
- // Keep track of the direct virtual bases.
- llvm::SmallPtrSet<CXXBaseSpecifier *, 16> DirectVBases;
- for (auto &I : ClassDecl->bases()) {
- if (I.isVirtual())
- DirectVBases.insert(&I);
- }
-
- // Push virtual bases before others.
- for (auto &VBase : ClassDecl->vbases()) {
- if (CXXCtorInitializer *Value
- = Info.AllBaseFields.lookup(VBase.getType()->getAs<RecordType>())) {
- // [class.base.init]p7, per DR257:
- // A mem-initializer where the mem-initializer-id names a virtual base
- // class is ignored during execution of a constructor of any class that
- // is not the most derived class.
- if (ClassDecl->isAbstract()) {
- // FIXME: Provide a fixit to remove the base specifier. This requires
- // tracking the location of the associated comma for a base specifier.
- Diag(Value->getSourceLocation(), diag::warn_abstract_vbase_init_ignored)
- << VBase.getType() << ClassDecl;
- DiagnoseAbstractType(ClassDecl);
- }
+ if (!HasDependentBasesOrInits) {
+ // Keep track of the direct virtual bases.
+ llvm::SmallPtrSet<CXXBaseSpecifier *, 16> DirectVBases;
+ for (auto &I : ClassDecl->bases()) {
+ if (I.isVirtual())
+ DirectVBases.insert(&I);
+ }
+
+ // Push virtual bases before others.
+ for (auto &VBase : ClassDecl->vbases()) {
+ if (CXXCtorInitializer *Value =
+ Info.AllBaseFields.lookup(VBase.getType()->getAs<RecordType>())) {
+ // [class.base.init]p7, per DR257:
+ // A mem-initializer where the mem-initializer-id names a virtual base
+ // class is ignored during execution of a constructor of any class
+ // that is not the most derived class.
+ if (ClassDecl->isAbstract()) {
+ // FIXME: Provide a fixit to remove the base specifier. This requires
+ // tracking the location of the associated comma for a base specifier.
+ Diag(Value->getSourceLocation(),
+ diag::warn_abstract_vbase_init_ignored)
+ << VBase.getType() << ClassDecl;
+ DiagnoseAbstractType(ClassDecl);
+ }
- Info.AllToInit.push_back(Value);
- } else if (!AnyErrors && !ClassDecl->isAbstract()) {
- // [class.base.init]p8, per DR257:
- // If a given [...] base class is not named by a mem-initializer-id
- // [...] and the entity is not a virtual base class of an abstract
- // class, then [...] the entity is default-initialized.
- bool IsInheritedVirtualBase = !DirectVBases.count(&VBase);
- CXXCtorInitializer *CXXBaseInit;
- if (BuildImplicitBaseInitializer(*this, Constructor, Info.IIK,
- &VBase, IsInheritedVirtualBase,
- CXXBaseInit)) {
- HadError = true;
- continue;
- }
+ Info.AllToInit.push_back(Value);
+ } else if (!AnyErrors && !ClassDecl->isAbstract()) {
+ // [class.base.init]p8, per DR257:
+ // If a given [...] base class is not named by a mem-initializer-id
+ // [...] and the entity is not a virtual base class of an abstract
+ // class, then [...] the entity is default-initialized.
+ bool IsInheritedVirtualBase = !DirectVBases.count(&VBase);
+ CXXCtorInitializer *CXXBaseInit;
+ if (BuildImplicitBaseInitializer(*this, Constructor, Info.IIK, &VBase,
+ IsInheritedVirtualBase, CXXBaseInit)) {
+ HadError = true;
+ continue;
+ }
- Info.AllToInit.push_back(CXXBaseInit);
+ Info.AllToInit.push_back(CXXBaseInit);
+ }
}
- }
- // Non-virtual bases.
- for (auto &Base : ClassDecl->bases()) {
- // Virtuals are in the virtual base list and already constructed.
- if (Base.isVirtual())
- continue;
-
- if (CXXCtorInitializer *Value
- = Info.AllBaseFields.lookup(Base.getType()->getAs<RecordType>())) {
- Info.AllToInit.push_back(Value);
- } else if (!AnyErrors) {
- CXXCtorInitializer *CXXBaseInit;
- if (BuildImplicitBaseInitializer(*this, Constructor, Info.IIK,
- &Base, /*IsInheritedVirtualBase=*/false,
- CXXBaseInit)) {
- HadError = true;
+ // Non-virtual bases.
+ for (auto &Base : ClassDecl->bases()) {
+ // Virtuals are in the virtual base list and already constructed.
+ if (Base.isVirtual())
continue;
- }
- Info.AllToInit.push_back(CXXBaseInit);
+ if (CXXCtorInitializer *Value =
+ Info.AllBaseFields.lookup(Base.getType()->getAs<RecordType>())) {
+ Info.AllToInit.push_back(Value);
+ } else if (!AnyErrors) {
+ CXXCtorInitializer *CXXBaseInit;
+ if (BuildImplicitBaseInitializer(*this, Constructor, Info.IIK, &Base,
+ /*IsInheritedVirtualBase=*/false,
+ CXXBaseInit)) {
+ HadError = true;
+ continue;
+ }
+
+ Info.AllToInit.push_back(CXXBaseInit);
+ }
}
}
@@ -4014,13 +4019,30 @@
}
}
- unsigned NumInitializers = Info.AllToInit.size();
+ // Leave all the initializers that didn't correspond to bases or fields in
+ // source order in the AST at the front. They must all be base initializers
+ // that are somehow dependent on template parameters.
+ llvm::SmallPtrSet<CXXCtorInitializer *, 16> UsedInits(Info.AllToInit.begin(),
+ Info.AllToInit.end());
+ SmallVector<CXXCtorInitializer *, 4> MissingInits;
+ for (CXXCtorInitializer *Init : Initializers) {
+ if (!UsedInits.count(Init)) {
+ assert(Init->isBaseInitializer() && "lost field initializer");
+ MissingInits.push_back(Init);
+ }
+ }
+
+ unsigned NumInitializers = MissingInits.size() + Info.AllToInit.size();
+ assert(NumInitializers >= Initializers.size() && "lost initializers");
if (NumInitializers > 0) {
Constructor->setNumCtorInitializers(NumInitializers);
CXXCtorInitializer **baseOrMemberInitializers =
new (Context) CXXCtorInitializer*[NumInitializers];
- memcpy(baseOrMemberInitializers, Info.AllToInit.data(),
- NumInitializers * sizeof(CXXCtorInitializer*));
+ memcpy(baseOrMemberInitializers, MissingInits.data(),
+ MissingInits.size() * sizeof(CXXCtorInitializer *));
+ memcpy(baseOrMemberInitializers + MissingInits.size(),
+ Info.AllToInit.data(),
+ Info.AllToInit.size() * sizeof(CXXCtorInitializer *));
Constructor->setCtorInitializers(baseOrMemberInitializers);
// Constructors implicitly reference the base and member
@@ -4059,9 +4081,6 @@
static void DiagnoseBaseOrMemInitializerOrder(
Sema &SemaRef, const CXXConstructorDecl *Constructor,
ArrayRef<CXXCtorInitializer *> Inits) {
- if (Constructor->getDeclContext()->isDependentContext())
- return;
-
// Don't check initializers order unless the warning is enabled at the
// location of at least one initializer.
bool ShouldCheckOrder = false;
@@ -4106,8 +4125,27 @@
unsigned IdealIndex = 0;
CXXCtorInitializer *PrevInit = nullptr;
- for (unsigned InitIndex = 0; InitIndex != Inits.size(); ++InitIndex) {
- CXXCtorInitializer *Init = Inits[InitIndex];
+
+ // If we have dependent bases or base initializers, we can't reason about
+ // their relative ordering. We will reorder all base initializers to the
+ // front, but we won't reorder them relative to each other.
+ bool HasDependentBasesOrInits =
+ hasDependentBasesOrInitializers(ClassDecl, Inits);
+
+ for (CXXCtorInitializer *Init : Inits) {
+ // Don't try to reason about the relative order of dependent type base
+ // initializers. We'll try to catch them during instantiation. We can still
+ // warn if this is a base initializer after a field initializer.
+ if (HasDependentBasesOrInits && Init->isBaseInitializer()) {
+ if (PrevInit && PrevInit->isAnyMemberInitializer()) {
+ SemaRef.Diag(PrevInit->getSourceLocation(),
+ diag::warn_initializer_out_of_order)
+ << /*field*/ 0 << PrevInit->getAnyMember()->getDeclName()
+ << /*base*/ 1 << Init->getTypeSourceInfo()->getType();
+ }
+ continue;
+ }
+
const void *InitKey = GetKeyForMember(SemaRef.Context, Init);
// Scan forward to try to find this initializer in the idealized
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
