Handle use of default member initializers before end of outermost class

Specifically, when we have this situation:
  struct A {
    template <typename T> struct B {
      int m1 = sizeof(A);
    };
    B<int> m2;
  };

We can't parse m1's initializer eagerly because we need A to be
complete.  Therefore we wait until the end of A's class scope to parse
it. However, we can trigger instantiation of B before the end of A,
which will attempt to instantiate the field decls eagerly, and it would
build a bad field decl instantiation that said it had an initializer but
actually lacked one.

Fixed by deferring instantiation of default member initializers until
they are needed during constructor analysis. This addresses a long
standing FIXME in the code.

Fixes PR19195.

Reviewed By: rsmith

Differential Revision: http://reviews.llvm.org/D5690

llvm-svn: 222192

5 files changed, 174 insertions, 65 deletions

diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index 014e3f9ef55..a46787fb0e8 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1441,6 +1441,17 @@ DeclContext *DeclContext::getEnclosingNamespaceContext() {
   return Ctx->getPrimaryContext();
 }
 
+RecordDecl *DeclContext::getOuterLexicalRecordContext() {
+  // Loop until we find a non-record context.
+  RecordDecl *OutermostRD = nullptr;
+  DeclContext *DC = this;
+  while (DC->isRecord()) {
+    OutermostRD = cast<RecordDecl>(DC);
+    DC = DC->getLexicalParent();
+  }
+  return OutermostRD;
+}
+
 bool DeclContext::InEnclosingNamespaceSetOf(const DeclContext *O) const {
   // For non-file contexts, this is equivalent to Equals.
   if (!isFileContext())
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index ab37e7a1138..23c5dfa98ec 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -2985,11 +2985,13 @@ bool Expr::HasSideEffects(const ASTContext &Ctx) const {
   case CXXDefaultArgExprClass:
     return cast<CXXDefaultArgExpr>(this)->getExpr()->HasSideEffects(Ctx);
 
-  case CXXDefaultInitExprClass:
-    if (const Expr *E = cast<CXXDefaultInitExpr>(this)->getExpr())
+  case CXXDefaultInitExprClass: {
+    const FieldDecl *FD = cast<CXXDefaultInitExpr>(this)->getField();
+    if (const Expr *E = FD->getInClassInitializer())
       return E->HasSideEffects(Ctx);
     // If we've not yet parsed the initializer, assume it has side-effects.
     return true;
+  }
 
   case CXXDynamicCastExprClass: {
     // A dynamic_cast expression has side-effects if it can throw.
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index c960f54b94a..6e9e7a83752 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -36,6 +36,7 @@
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/Template.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include <map>
@@ -3730,19 +3731,19 @@ static bool CollectFieldInitializer(Sema &SemaRef, BaseAndFieldInfo &Info,
     return false;
 
   if (Field->hasInClassInitializer() && !Info.isImplicitCopyOrMove()) {
-    Expr *DIE = CXXDefaultInitExpr::Create(SemaRef.Context,
-                                           Info.Ctor->getLocation(), Field);
+    ExprResult DIE =
+        SemaRef.BuildCXXDefaultInitExpr(Info.Ctor->getLocation(), Field);
+    if (DIE.isInvalid())
+      return true;
     CXXCtorInitializer *Init;
     if (Indirect)
-      Init = new (SemaRef.Context) CXXCtorInitializer(SemaRef.Context, Indirect,
-                                                      SourceLocation(),
-                                                      SourceLocation(), DIE,
-                                                      SourceLocation());
+      Init = new (SemaRef.Context)
+          CXXCtorInitializer(SemaRef.Context, Indirect, SourceLocation(),
+                             SourceLocation(), DIE.get(), SourceLocation());
     else
-      Init = new (SemaRef.Context) CXXCtorInitializer(SemaRef.Context, Field,
-                                                      SourceLocation(),
-                                                      SourceLocation(), DIE,
-                                                      SourceLocation());
+      Init = new (SemaRef.Context)
+          CXXCtorInitializer(SemaRef.Context, Field, SourceLocation(),
+                             SourceLocation(), DIE.get(), SourceLocation());
     return Info.addFieldInitializer(Init);
   }
 
@@ -8579,22 +8580,6 @@ Sema::ComputeDefaultedDefaultCtorExceptionSpec(SourceLocation Loc,
     if (F->hasInClassInitializer()) {
       if (Expr *E = F->getInClassInitializer())
         ExceptSpec.CalledExpr(E);
-      else if (!F->isInvalidDecl())
-        // DR1351:
-        //   If the brace-or-equal-initializer of a non-static data member
-        //   invokes a defaulted default constructor of its class or of an
-        //   enclosing class in a potentially evaluated subexpression, the
-        //   program is ill-formed.
-        //
-        // This resolution is unworkable: the exception specification of the
-        // default constructor can be needed in an unevaluated context, in
-        // particular, in the operand of a noexcept-expression, and we can be
-        // unable to compute an exception specification for an enclosed class.
-        //
-        // We do not allow an in-class initializer to require the evaluation
-        // of the exception specification for any in-class initializer whose
-        // definition is not lexically complete.
-        Diag(Loc, diag::err_in_class_initializer_references_def_ctor) << MD;
     } else if (const RecordType *RecordTy
               = Context.getBaseElementType(F->getType())->getAs<RecordType>()) {
       CXXRecordDecl *FieldRecDecl = cast<CXXRecordDecl>(RecordTy->getDecl());
@@ -8662,9 +8647,6 @@ Sema::ComputeInheritingCtorExceptionSpec(CXXConstructorDecl *CD) {
     if (F->hasInClassInitializer()) {
       if (Expr *E = F->getInClassInitializer())
         ExceptSpec.CalledExpr(E);
-      else if (!F->isInvalidDecl())
-        Diag(CD->getLocation(),
-             diag::err_in_class_initializer_references_def_ctor) << CD;
     } else if (const RecordType *RecordTy
               = Context.getBaseElementType(F->getType())->getAs<RecordType>()) {
       CXXRecordDecl *FieldRecDecl = cast<CXXRecordDecl>(RecordTy->getDecl());
@@ -11129,6 +11111,56 @@ Sema::BuildCXXConstructExpr(SourceLocation ConstructLoc, QualType DeclInitType,
       ParenRange);
 }
 
+ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
+  assert(Field->hasInClassInitializer());
+
+  // If we already have the in-class initializer nothing needs to be done.
+  if (Field->getInClassInitializer())
+    return CXXDefaultInitExpr::Create(Context, Loc, Field);
+
+  // Maybe we haven't instantiated the in-class initializer. Go check the
+  // pattern FieldDecl to see if it has one.
+  CXXRecordDecl *ParentRD = cast<CXXRecordDecl>(Field->getParent());
+
+  if (isTemplateInstantiation(ParentRD->getTemplateSpecializationKind())) {
+    CXXRecordDecl *ClassPattern = ParentRD->getTemplateInstantiationPattern();
+    DeclContext::lookup_result Lookup =
+        ClassPattern->lookup(Field->getDeclName());
+    assert(Lookup.size() == 1);
+    FieldDecl *Pattern = cast<FieldDecl>(Lookup[0]);
+    if (InstantiateInClassInitializer(Loc, Field, Pattern,
+                                      getTemplateInstantiationArgs(Field)))
+      return ExprError();
+    return CXXDefaultInitExpr::Create(Context, Loc, Field);
+  }
+
+  // DR1351:
+  //   If the brace-or-equal-initializer of a non-static data member
+  //   invokes a defaulted default constructor of its class or of an
+  //   enclosing class in a potentially evaluated subexpression, the
+  //   program is ill-formed.
+  //
+  // This resolution is unworkable: the exception specification of the
+  // default constructor can be needed in an unevaluated context, in
+  // particular, in the operand of a noexcept-expression, and we can be
+  // unable to compute an exception specification for an enclosed class.
+  //
+  // Any attempt to resolve the exception specification of a defaulted default
+  // constructor before the initializer is lexically complete will ultimately
+  // come here at which point we can diagnose it.
+  RecordDecl *OutermostClass = ParentRD->getOuterLexicalRecordContext();
+  if (OutermostClass == ParentRD) {
+    Diag(Field->getLocEnd(), diag::err_in_class_initializer_not_yet_parsed)
+        << ParentRD << Field;
+  } else {
+    Diag(Field->getLocEnd(),
+         diag::err_in_class_initializer_not_yet_parsed_outer_class)
+        << ParentRD << OutermostClass << Field;
+  }
+
+  return ExprError();
+}
+
 void Sema::FinalizeVarWithDestructor(VarDecl *VD, const RecordType *Record) {
   if (VD->isInvalidDecl()) return;
 
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 39d6a575aa2..b57e029ddfd 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -466,11 +466,15 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
     //   members in the aggregate, then each member not explicitly initialized
     //   shall be initialized from its brace-or-equal-initializer [...]
     if (Field->hasInClassInitializer()) {
-      Expr *DIE = CXXDefaultInitExpr::Create(SemaRef.Context, Loc, Field);
+      ExprResult DIE = SemaRef.BuildCXXDefaultInitExpr(Loc, Field);
+      if (DIE.isInvalid()) {
+        hadError = true;
+        return;
+      }
       if (Init < NumInits)
-        ILE->setInit(Init, DIE);
+        ILE->setInit(Init, DIE.get());
       else {
-        ILE->updateInit(SemaRef.Context, Init, DIE);
+        ILE->updateInit(SemaRef.Context, Init, DIE.get());
         RequiresSecondPass = true;
       }
       return;
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index a9aae009401..891cf734c36 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -448,6 +448,10 @@ void Sema::PrintInstantiationStack() {
                      diag::note_template_enum_def_here)
           << ED
           << Active->InstantiationRange;
+      } else if (FieldDecl *FD = dyn_cast<FieldDecl>(D)) {
+        Diags.Report(Active->PointOfInstantiation,
+                     diag::note_template_nsdmi_here)
+            << FD << Active->InstantiationRange;
       } else {
         Diags.Report(Active->PointOfInstantiation,
                      diag::note_template_type_alias_instantiation_here)
@@ -1997,8 +2001,6 @@ Sema::InstantiateClass(SourceLocation PointOfInstantiation,
 
   TemplateDeclInstantiator Instantiator(*this, Instantiation, TemplateArgs);
   SmallVector<Decl*, 4> Fields;
-  SmallVector<std::pair<FieldDecl*, FieldDecl*>, 4>
-    FieldsWithMemberInitializers;
   // Delay instantiation of late parsed attributes.
   LateInstantiatedAttrVec LateAttrs;
   Instantiator.enableLateAttributeInstantiation(&LateAttrs);
@@ -2025,10 +2027,6 @@ Sema::InstantiateClass(SourceLocation PointOfInstantiation,
     if (NewMember) {
       if (FieldDecl *Field = dyn_cast<FieldDecl>(NewMember)) {
         Fields.push_back(Field);
-        FieldDecl *OldField = cast<FieldDecl>(Member);
-        if (OldField->getInClassInitializer())
-          FieldsWithMemberInitializers.push_back(std::make_pair(OldField,
-                                                                Field));
       } else if (EnumDecl *Enum = dyn_cast<EnumDecl>(NewMember)) {
         // C++11 [temp.inst]p1: The implicit instantiation of a class template
         // specialization causes the implicit instantiation of the definitions
@@ -2065,31 +2063,6 @@ Sema::InstantiateClass(SourceLocation PointOfInstantiation,
               SourceLocation(), SourceLocation(), nullptr);
   CheckCompletedCXXClass(Instantiation);
 
-  // Attach any in-class member initializers now the class is complete.
-  // FIXME: We are supposed to defer instantiating these until they are needed.
-  if (!FieldsWithMemberInitializers.empty()) {
-    // C++11 [expr.prim.general]p4:
-    //   Otherwise, if a member-declarator declares a non-static data member 
-    //  (9.2) of a class X, the expression this is a prvalue of type "pointer
-    //  to X" within the optional brace-or-equal-initializer. It shall not 
-    //  appear elsewhere in the member-declarator.
-    CXXThisScopeRAII ThisScope(*this, Instantiation, (unsigned)0);
-    
-    for (unsigned I = 0, N = FieldsWithMemberInitializers.size(); I != N; ++I) {
-      FieldDecl *OldField = FieldsWithMemberInitializers[I].first;
-      FieldDecl *NewField = FieldsWithMemberInitializers[I].second;
-      Expr *OldInit = OldField->getInClassInitializer();
-
-      ActOnStartCXXInClassMemberInitializer();
-      ExprResult NewInit = SubstInitializer(OldInit, TemplateArgs,
-                                            /*CXXDirectInit=*/false);
-      Expr *Init = NewInit.get();
-      assert((!Init || !isa<ParenListExpr>(Init)) &&
-             "call-style init in class");
-      ActOnFinishCXXInClassMemberInitializer(NewField,
-        Init ? Init->getLocStart() : SourceLocation(), Init);
-    }
-  }
   // Instantiate late parsed attributes, and attach them to their decls.
   // See Sema::InstantiateAttrs
   for (LateInstantiatedAttrVec::iterator I = LateAttrs.begin(),
@@ -2230,6 +2203,80 @@ bool Sema::InstantiateEnum(SourceLocation PointOfInstantiation,
   return Instantiation->isInvalidDecl();
 }
 
+
+/// \brief Instantiate the definition of a field from the given pattern.
+///
+/// \param PointOfInstantiation The point of instantiation within the
+///        source code.
+/// \param Instantiation is the declaration whose definition is being
+///        instantiated. This will be a class of a class temploid
+///        specialization, or a local enumeration within a function temploid
+///        specialization.
+/// \param Pattern The templated declaration from which the instantiation
+///        occurs.
+/// \param TemplateArgs The template arguments to be substituted into
+///        the pattern.
+///
+/// \return \c true if an error occurred, \c false otherwise.
+bool Sema::InstantiateInClassInitializer(
+    SourceLocation PointOfInstantiation, FieldDecl *Instantiation,
+    FieldDecl *Pattern, const MultiLevelTemplateArgumentList &TemplateArgs) {
+  // If there is no initializer, we don't need to do anything.
+  if (!Pattern->hasInClassInitializer())
+    return false;
+
+  assert(Instantiation->getInClassInitStyle() ==
+             Pattern->getInClassInitStyle() &&
+         "pattern and instantiation disagree about init style");
+
+  // Error out if we haven't parsed the initializer of the pattern yet because
+  // we are waiting for the closing brace of the outer class.
+  Expr *OldInit = Pattern->getInClassInitializer();
+  if (!OldInit) {
+    RecordDecl *PatternRD = Pattern->getParent();
+    RecordDecl *OutermostClass = PatternRD->getOuterLexicalRecordContext();
+    if (OutermostClass == PatternRD) {
+      Diag(Pattern->getLocEnd(), diag::err_in_class_initializer_not_yet_parsed)
+          << PatternRD << Pattern;
+    } else {
+      Diag(Pattern->getLocEnd(),
+           diag::err_in_class_initializer_not_yet_parsed_outer_class)
+          << PatternRD << OutermostClass << Pattern;
+    }
+    Instantiation->setInvalidDecl();
+    return true;
+  }
+
+  InstantiatingTemplate Inst(*this, PointOfInstantiation, Instantiation);
+  if (Inst.isInvalid())
+    return true;
+
+  // Enter the scope of this instantiation. We don't use PushDeclContext because
+  // we don't have a scope.
+  ContextRAII SavedContext(*this, Instantiation->getParent());
+  EnterExpressionEvaluationContext EvalContext(*this,
+                                               Sema::PotentiallyEvaluated);
+
+  LocalInstantiationScope Scope(*this);
+
+  // Instantiate the initializer.
+  ActOnStartCXXInClassMemberInitializer();
+  CXXThisScopeRAII ThisScope(*this, Instantiation->getParent(), /*TypeQuals=*/0);
+
+  ExprResult NewInit = SubstInitializer(OldInit, TemplateArgs,
+                                        /*CXXDirectInit=*/false);
+  Expr *Init = NewInit.get();
+  assert((!Init || !isa<ParenListExpr>(Init)) && "call-style init in class");
+  ActOnFinishCXXInClassMemberInitializer(
+      Instantiation, Init ? Init->getLocStart() : SourceLocation(), Init);
+
+  // Exit the scope of this instantiation.
+  SavedContext.pop();
+
+  // Return true if the in-class initializer is still missing.
+  return !Instantiation->getInClassInitializer();
+}
+
 namespace {
   /// \brief A partial specialization whose template arguments have matched
   /// a given template-id.
@@ -2591,6 +2638,19 @@ Sema::InstantiateClassMembers(SourceLocation PointOfInstantiation,
         MSInfo->setTemplateSpecializationKind(TSK);
         MSInfo->setPointOfInstantiation(PointOfInstantiation);
       }
+    } else if (auto *Field = dyn_cast<FieldDecl>(D)) {
+      // No need to instantiate in-class initializers during explicit
+      // instantiation.
+      if (Field->hasInClassInitializer() && TSK == TSK_ImplicitInstantiation) {
+        CXXRecordDecl *ClassPattern =
+            Instantiation->getTemplateInstantiationPattern();
+        DeclContext::lookup_result Lookup =
+            ClassPattern->lookup(Field->getDeclName());
+        assert(Lookup.size() == 1);
+        FieldDecl *Pattern = cast<FieldDecl>(Lookup[0]);
+        InstantiateInClassInitializer(PointOfInstantiation, Field, Pattern,
+                                      TemplateArgs);
+      }
     }
   }
 }