Teach ActOnMemInitializers to cope with dependent bases and initializers

[rnk]

Summary

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

lib/Sema/SemaDeclCXX.cpp

if (isIncompleteOrZeroLengthArrayType(SemaRef.Context, Field->getType()))
     return false;
 
   // 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).
   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))
     return true;
 
   if (!Init)
     return false;
 
Sema::SetDelegatingInitializer(CXXConstructorDecl *Constructor,
 
   DelegatingCtorDecls.push_back(Constructor);
 
   DiagnoseUninitializedFields(*this, Constructor);
 
   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;
-  }
+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;
 
+bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors,
+                               ArrayRef<CXXCtorInitializer *> Initializers) {
   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;
-  
+  // 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;
 
-  for (unsigned i = 0; i < Initializers.size(); i++) {
-    CXXCtorInitializer *Member = Initializers[i];
-
-    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()) {
         for (auto *C : F->chain()) {
           FieldDecl *FD = dyn_cast<FieldDecl>(C);
           if (FD && FD->getParent()->isUnion())
             Info.ActiveUnionMember.insert(std::make_pair(
                 FD->getParent()->getCanonicalDecl(), FD->getCanonicalDecl()));
         }
       } else if (FieldDecl *FD = Member->getMember()) {
         if (FD->getParent()->isUnion())
           Info.ActiveUnionMember.insert(std::make_pair(
               FD->getParent()->getCanonicalDecl(), FD->getCanonicalDecl()));
       }
     }
   }
 
+  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>())) {
+      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.
+        //   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)
+          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)) {
+        if (BuildImplicitBaseInitializer(*this, Constructor, Info.IIK, &VBase,
+                                         IsInheritedVirtualBase, CXXBaseInit)) {
           HadError = true;
           continue;
         }
 
         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>())) {
+      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,
+        if (BuildImplicitBaseInitializer(*this, Constructor, Info.IIK, &Base,
+                                         /*IsInheritedVirtualBase=*/false,
                                          CXXBaseInit)) {
           HadError = true;
           continue;
         }
 
         Info.AllToInit.push_back(CXXBaseInit);
       }
     }
+  }
 
   // Fields.
   for (auto *Mem : ClassDecl->decls()) {
     if (auto *F = dyn_cast<FieldDecl>(Mem)) {
       // C++ [class.bit]p2:
       //   A declaration for a bit-field that omits the identifier declares an
       //   unnamed bit-field. Unnamed bit-fields are not members and cannot be
       //   initialized.
if (auto *F = dyn_cast<IndirectFieldDecl>(Mem)) {
       // Initialize each field of an anonymous struct individually.
       if (CollectFieldInitializer(*this, Info, F->getAnonField(), F))
         HadError = true;
       
       continue;        
     }
   }
 
-  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
     // destructors.
     MarkBaseAndMemberDestructorsReferenced(Constructor->getLocation(),
                                            Constructor->getParent());
   }
 
if (!Member->isAnyMemberInitializer())
     return GetKeyForBase(Context, QualType(Member->getBaseClass(), 0));
     
   return Member->getAnyMember()->getCanonicalDecl();
 }
 
 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;
   for (unsigned InitIndex = 0; InitIndex != Inits.size(); ++InitIndex) {
     CXXCtorInitializer *Init = Inits[InitIndex];
     if (!SemaRef.Diags.isIgnored(diag::warn_initializer_out_of_order,
                                  Init->getSourceLocation())) {
       ShouldCheckOrder = true;
for (auto *Field : ClassDecl->fields()) {
     
     PopulateKeysForFields(Field, IdealInitKeys);
   }
   
   unsigned NumIdealInits = IdealInitKeys.size();
   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
     // initializers list.
     for (; IdealIndex != NumIdealInits; ++IdealIndex)
       if (InitKey == IdealInitKeys[IdealIndex])
         break;
 

test/SemaTemplate/class-template-ctor-initializer.cpp

namespace PR7259 {
   main (void)
   {
     Final final;
     return 0;
   }
 }
 
 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;
 }

test/SemaTemplate/default-expr-arguments.cpp

namespace pr5301 {
 
   void test() {
     h(0);
   }
 }
 
 // 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>
+  // 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
   
   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}}
   };
   
   struct A { };
   struct B { };
 
   template<typename>
   void FilterVTs() {
     vector<A> Result;
   }
   
   void f() {
     vector<A> Result;
   }
 
   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() { }
   };
 
   void f2() {
     X<float> x; // expected-note{{member function}}
   }
 }
 

test/SemaTemplate/instantiate-member-initializers.cpp

 template<typename T> struct B {
   B() : b(1), // expected-warning {{field 'b' will be initialized after field 'a'}}
     a(2) { }
   
   int a;
   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> {
 public:
   BB() : AA<T>(1) {}
 };
 BB<int> x;