[PATCH] Handle use of default member initializers before end of outermost class
Richard Smith
richard at metafoo.co.uk
Thu Oct 9 16:52:27 PDT 2014
================
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
More information about the cfe-commits
mailing list