================
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">;
----------------
rsmith wrote:
> "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.
The other diagnostic was much more specific, but it fired too late, so we ended
up with both diagnostics.
================
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
----------------
rsmith wrote:
> 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.
I wonder if r218466 is related: "Move calls to ResolveExceptionSpec into
DefineImplicit*".
================
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 =
----------------
rsmith wrote:
> 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`.
I think this is addressed now. I can't test the explicit and partial
specialization cases because I can't seem to declare such specializations
before leaving class scope, which hides this codepath.
================
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;
+ }
----------------
rsmith wrote:
> 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.
Should be fixed with DeclContext::getOuterLexicalRecordContext().
================
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);
+ }
}
----------------
rsmith wrote:
> Do we still set the 'this field has an initializer' flag somewhere?
Yes, TemplateDeclInstantiator::VisitFieldDecl() copies it from the pattern to
the instantiation.
================
Comment at: lib/Sema/SemaTemplateInstantiate.cpp:2199
@@ +2198,3 @@
+bool
+Sema::InstantiateField(SourceLocation PointOfInstantiation,
+ FieldDecl *Instantiation, FieldDecl *Pattern,
----------------
rsmith wrote:
> 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.
Feel free to pick one if you like it:
InstantiateFieldInitializer
InstantiateDefaultInitializer
InstantiateInClassInitializer
I'm going with InstantiateInClassInitializer for now, since it's grep-able in
relation to InClassInitializer.
================
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;
+}
----------------
rsmith wrote:
> 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).
Nice! This fixed everything and allowed me to completely defer instantiation of
NSDMIs until we form CXXDefaultInitExprs.
http://reviews.llvm.org/D5690
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
