[PATCH] Handle use of default member initializers before end of outermost class

[Richard Smith]

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6189
@@ +6188,3 @@
+def err_in_class_initializer_not_yet_parsed : Error<
+  "cannot default initialize non-static data member %0 before the end of the "
+  "outermost containing class %1">;
----------------
"default initialize" is the wrong term here; it means something else.

The existing diagnostic was specifically targeted at http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1397, and its wording was intended to explain what the relevant rule is. I'm not sure how best to update the diagnostic text, but this wording needs more work.

================
Comment at: lib/Sema/SemaDeclCXX.cpp:8420-8435
@@ -8419,18 +8420,2 @@
         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
----------------
You can trigger the computation of the exception specification for a defaulted constructor without causing it to be defined (or at least, in principle; it looks like Clang doesn't get this right). If you do so from a default initializer, we're required to reject the program under DR1397, and we shouldn't just ignore the fact that there was a default initializer.

================
Comment at: lib/Sema/SemaDeclCXX.cpp:10979-10981
@@ +10978,5 @@
+  if (isTemplateInstantiation(ParentRD->getTemplateSpecializationKind())) {
+    auto *InstantiatedClass = cast<ClassTemplateSpecializationDecl>(ParentRD);
+    CXXRecordDecl *ClassPattern =
+        InstantiatedClass->getSpecializedTemplate()->getTemplatedDecl();
+    DeclContext::lookup_result Lookup =
----------------
This isn't right. You need to handle two cases:

1) The class is an instantiation of a class template or partial specialization; cast to `ClassTemplateSpecializationDecl` and call `getInstantiatedFrom()`.
2) The class is a member of a class template (recursively), and is not a `ClassTemplateSpecializationDecl`. In this case, call `getInstantiatedFromMemberClass` to find the pattern.

In the first case, you may also need to walk from a member template or member partial specailization back to its pattern (and be careful to stop when you hit a member specialization). See `hasVisibleDefinition` in SemaType.cpp and `FunctionDecl::getTemplateInstantiationPattern` for examples of where we already do this.

It seems like a good idea to move this into a new `CXXRecordDecl::getTemplateInstantiationPattern`.

================
Comment at: lib/Sema/SemaDeclCXX.cpp:10992-10995
@@ +10991,6 @@
+  RecordDecl *OutermostClass = ParentRD;
+  for (DeclContext *DC = OutermostClass; !DC->isTranslationUnit();
+       DC = DC->getParent()) {
+    if (auto *RD = dyn_cast<RecordDecl>(DC))
+      OutermostClass = RD;
+  }
----------------
You should walk over lexically-enclosing contexts, and stop when you hit the first context that's not a `CXXRecordDecl`. For a local class, surrounding classes don't matter, and for an out-of-line definition of a nested class, its containing class doesn't matter.

================
Comment at: lib/Sema/SemaTemplateInstantiate.cpp:2033-2040
@@ -2032,10 +2032,10 @@
 
-      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);
+      // If we haven't parsed the initializer yet, defer instantiation until
+      // later.
+      // FIXME: If we can tolerate defering some initializers, it shouldn't be
+      // hard to defer *all* field instantiation until later as described above.
+      if (OldInit) {
+        InstantiateField(Instantiation->getLocation(), NewField, OldField,
+                         TemplateArgs);
+      }
     }
----------------
Do we still set the 'this field has an initializer' flag somewhere?

================
Comment at: lib/Sema/SemaTemplateInstantiate.cpp:2199
@@ +2198,3 @@
+bool
+Sema::InstantiateField(SourceLocation PointOfInstantiation,
+                       FieldDecl *Instantiation, FieldDecl *Pattern,
----------------
I'd prefer for this function to have a name related to in-class initializers, since that's the specific thing it's dealing with.

================
Comment at: lib/Sema/SemaTemplateInstantiate.cpp:2228-2235
@@ +2227,10 @@
+  // Instantiate the initializer.
+  ActOnStartCXXInClassMemberInitializer();
+  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);
+  return Init == nullptr;
+}
----------------
You should create an `InstantiatingTemplate` surrounding this for proper handling of instantiation backtraces. (Note that you're currently not using `PointOfInstantiation`.)

You also need a `ContextRAII` and `LocalInstantiationScope` here so that (for instance) lambdas within the initializer are created in the right `DeclContext`, and we perform proper access checking, and so on.

You should also `EnterExpressionEvaluationContext` to ensure we're in a `PotentiallyEvaluated` context (the instantiation could be triggered by an aggregate initialization in an unevaluated operand).

http://reviews.llvm.org/D5690