Hi rsmith,
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 doing template instantiation when building a CXXDefaultInitExpr
and erorring out if the initializer is missing for any other reason.
Fixes PR19195.
This does *not* defer all field instantiation until later, as that
causes assertions in the test suite. Fixing that might be the right
thing to do, but I wanted feedback on the approach so far first.
http://reviews.llvm.org/D5690
Files:
include/clang/AST/ExprCXX.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaInit.cpp
lib/Sema/SemaTemplateInstantiate.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
test/SemaCXX/constant-expression-cxx11.cpp
test/SemaCXX/implicit-exception-spec.cpp
test/SemaCXX/member-init.cpp
Index: include/clang/AST/ExprCXX.h
===================================================================
--- include/clang/AST/ExprCXX.h
+++ include/clang/AST/ExprCXX.h
@@ -967,8 +967,14 @@
const FieldDecl *getField() const { return Field; }
/// \brief Get the initialization expression that will be used.
- const Expr *getExpr() const { return Field->getInClassInitializer(); }
- Expr *getExpr() { return Field->getInClassInitializer(); }
+ const Expr *getExpr() const {
+ assert(Field->getInClassInitializer() && "initializer hasn't been parsed");
+ return Field->getInClassInitializer();
+ }
+ Expr *getExpr() {
+ assert(Field->getInClassInitializer() && "initializer hasn't been parsed");
+ return Field->getInClassInitializer();
+ }
SourceLocation getLocStart() const LLVM_READONLY { return Loc; }
SourceLocation getLocEnd() const LLVM_READONLY { return Loc; }
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6185,9 +6185,9 @@
"'constexpr' specifier">;
def err_in_class_initializer_non_constant : Error<
"in-class initializer for static data member is not a constant expression">;
-def err_in_class_initializer_references_def_ctor : Error<
- "defaulted default constructor of %0 cannot be used by non-static data "
- "member initializer which appears before end of class definition">;
+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">;
def ext_in_class_initializer_non_constant : Extension<
"in-class initializer for static data member is not a constant expression; "
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -3916,6 +3916,8 @@
bool IsStdInitListInitialization, bool RequiresZeroInit,
unsigned ConstructKind, SourceRange ParenRange);
+ ExprResult BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field);
+
/// BuildCXXDefaultArgExpr - Creates a CXXDefaultArgExpr, instantiating
/// the default expr if needed.
ExprResult BuildCXXDefaultArgExpr(SourceLocation CallLoc,
@@ -6731,6 +6733,10 @@
const MultiLevelTemplateArgumentList &TemplateArgs,
TemplateSpecializationKind TSK);
+ bool InstantiateField(SourceLocation PointOfInstantiation,
+ FieldDecl *Instantiation, FieldDecl *Pattern,
+ const MultiLevelTemplateArgumentList &TemplateArgs);
+
struct LateInstantiatedAttribute {
const Attr *TmplAttr;
LocalInstantiationScope *Scope;
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ 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>
@@ -3610,19 +3611,19 @@
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);
}
@@ -8417,22 +8418,6 @@
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());
@@ -8500,9 +8485,6 @@
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());
@@ -10967,6 +10949,57 @@
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);
+
+ // 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 evaluate 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.
+
+ // 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())) {
+ auto *InstantiatedClass = cast<ClassTemplateSpecializationDecl>(ParentRD);
+ CXXRecordDecl *ClassPattern =
+ InstantiatedClass->getSpecializedTemplate()->getTemplatedDecl();
+ DeclContext::lookup_result Lookup =
+ ClassPattern->lookup(Field->getDeclName());
+ assert(Lookup.size() == 1);
+ FieldDecl *Pattern = cast<FieldDecl>(Lookup[0]);
+ if (InstantiateField(Loc, Field, Pattern,
+ getTemplateInstantiationArgs(Field)))
+ return ExprError();
+ return CXXDefaultInitExpr::Create(Context, Loc, Field);
+ }
+ RecordDecl *OutermostClass = ParentRD;
+ for (DeclContext *DC = OutermostClass; !DC->isTranslationUnit();
+ DC = DC->getParent()) {
+ if (auto *RD = dyn_cast<RecordDecl>(DC))
+ OutermostClass = RD;
+ }
+ Diag(Field->getLocEnd(), diag::err_in_class_initializer_not_yet_parsed)
+ << Field << OutermostClass;
+
+ return ExprError();
+}
+
void Sema::FinalizeVarWithDestructor(VarDecl *VD, const RecordType *Record) {
if (VD->isInvalidDecl()) return;
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -466,11 +466,15 @@
// 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;
Index: lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -1976,7 +1976,7 @@
if (FieldDecl *Field = dyn_cast<FieldDecl>(NewMember)) {
Fields.push_back(Field);
FieldDecl *OldField = cast<FieldDecl>(Member);
- if (OldField->getInClassInitializer())
+ if (OldField->hasInClassInitializer())
FieldsWithMemberInitializers.push_back(std::make_pair(OldField,
Field));
} else if (EnumDecl *Enum = dyn_cast<EnumDecl>(NewMember)) {
@@ -2030,14 +2030,14 @@
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);
+ // 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);
+ }
}
}
// Instantiate late parsed attributes, and attach them to their decls.
@@ -2180,6 +2180,61 @@
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::InstantiateField(SourceLocation PointOfInstantiation,
+ FieldDecl *Instantiation, FieldDecl *Pattern,
+ const MultiLevelTemplateArgumentList &TemplateArgs) {
+ // We only need to instantiate the initializer, the rest of the field was
+ // instantiated during InstantiateClass.
+ 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) {
+ // FIXME: Deduplicate code with SemaInit
+ RecordDecl *OutermostClass = Pattern->getParent();
+ for (DeclContext *DC = OutermostClass; !DC->isTranslationUnit();
+ DC = DC->getParent()) {
+ if (auto *RD = dyn_cast<RecordDecl>(DC))
+ OutermostClass = RD;
+ }
+ Diag(Pattern->getLocEnd(), diag::err_in_class_initializer_not_yet_parsed)
+ << Pattern << OutermostClass;
+ Instantiation->setInvalidDecl();
+ return true;
+ }
+
+ // 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;
+}
+
namespace {
/// \brief A partial specialization whose template arguments have matched
/// a given template-id.
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -548,6 +548,24 @@
return nullptr;
}
+#if 0
+ // Instantiate the in-class initializer if present.
+ llvm::errs() << "VisitFieldDecl\n";
+ D->dump();
+ if (D->hasInClassInitializer()) {
+ Expr *Init = D->getInClassInitializer();
+ llvm::errs() << "Init\n";
+ Init->dump();
+ SemaRef.ActOnStartCXXInClassMemberInitializer();
+ bool IsDirectInit = D->getInClassInitStyle() == ICIS_ListInit;
+ Init = SemaRef.SubstInitializer(Init, TemplateArgs, IsDirectInit).get();
+ llvm::errs() << "instantiated Init\n";
+ Init->dump();
+ SemaRef.ActOnFinishCXXInClassMemberInitializer(Field, Init->getLocStart(),
+ Init);
+ }
+#endif
+
SemaRef.InstantiateAttrs(TemplateArgs, D, Field, LateAttrs, StartingScope);
if (Field->hasAttrs())
Index: test/SemaCXX/constant-expression-cxx11.cpp
===================================================================
--- test/SemaCXX/constant-expression-cxx11.cpp
+++ test/SemaCXX/constant-expression-cxx11.cpp
@@ -1842,8 +1842,9 @@
namespace BadDefaultInit {
template<int N> struct X { static const int n = N; };
- struct A { // expected-note {{subexpression}}
- int k = X<A().k>::n; // expected-error {{defaulted default constructor of 'A' cannot be used}} expected-error {{not a constant expression}} expected-note {{in call to 'A()'}}
+ struct A {
+ int k = // expected-error {{cannot default initialize non-static data member 'k' before the end of the outermost containing class 'A'}}
+ X<A().k>::n; // expected-error {{not a constant expression}} expected-note {{implicit default constructor for 'BadDefaultInit::A' first required here}}
};
// FIXME: The "constexpr constructor must initialize all members" diagnostic
Index: test/SemaCXX/implicit-exception-spec.cpp
===================================================================
--- test/SemaCXX/implicit-exception-spec.cpp
+++ test/SemaCXX/implicit-exception-spec.cpp
@@ -17,31 +17,32 @@
// is false.
bool ThrowSomething() noexcept(false);
struct ConstExpr {
- bool b = noexcept(ConstExpr()) && ThrowSomething(); // expected-error {{cannot be used by non-static data member initializer}}
+ bool b = noexcept(ConstExpr()) && ThrowSomething(); // expected-error {{cannot default initialize non-static data member}}
+ // expected-note@-1 {{implicit default constructor for 'InClassInitializers::ConstExpr' first required here}}
};
- // We can use it now.
- bool w = noexcept(ConstExpr());
// Much more obviously broken: we can't parse the initializer without already
// knowing whether it produces a noexcept expression.
struct TemplateArg {
- int n = ExceptionIf<noexcept(TemplateArg())>::f(); // expected-error {{cannot be used by non-static data member initializer}}
+ int n = ExceptionIf<noexcept(TemplateArg())>::f(); // expected-error {{cannot default initialize non-static data member}}
+ // expected-note@-1 {{implicit default constructor for 'InClassInitializers::TemplateArg' first required here}}
};
- bool x = noexcept(TemplateArg());
// And within a nested class.
- struct Nested { // expected-error {{cannot be used by non-static data member initializer}}
+ struct Nested { // expected-note {{implicit default constructor for 'InClassInitializers::Nested::Inner' first required here}}
struct Inner {
+ // expected-error@+1 {{cannot default initialize non-static data member}}
int n = ExceptionIf<noexcept(Nested())>::f(); // expected-note {{implicit default constructor for 'InClassInitializers::Nested' first required here}}
} inner;
};
- struct Nested2 {
+ struct Nested2 { // expected-error {{implicit default constructor for 'InClassInitializers::Nested2' must explicitly initialize the member 'inner' which does not have a default constructor}}
struct Inner;
- int n = Inner().n; // expected-error {{cannot be used by non-static data member initializer}}
- struct Inner {
- int n = ExceptionIf<noexcept(Nested2())>::f();
- } inner;
+ int n = Inner().n; // expected-note {{implicit default constructor for 'InClassInitializers::Nested2::Inner' first required here}}
+ struct Inner { // expected-note {{declared here}}
+ int n = ExceptionIf<noexcept(Nested2())>::f(); // expected-error {{cannot default initialize non-static data member}}
+ // expected-note@-1 {{implicit default constructor for 'InClassInitializers::Nested2' first required here}}
+ } inner; // expected-note {{member is declared here}}
};
}
Index: test/SemaCXX/member-init.cpp
===================================================================
--- test/SemaCXX/member-init.cpp
+++ test/SemaCXX/member-init.cpp
@@ -14,7 +14,10 @@
bool b();
int k;
struct Recurse {
- int &n = b() ? Recurse().n : k; // expected-error {{defaulted default constructor of 'Recurse' cannot be used by non-static data member initializer which appears before end of class definition}}
+ int &n = // expected-error {{cannot default initialize non-static data member 'n' before the end of the outermost containing class 'Recurse'}}
+ b() ?
+ Recurse().n : // expected-note {{implicit default constructor for 'Recurse' first required here}}
+ k;
};
struct UnknownBound {
@@ -110,3 +113,35 @@
struct Y { int b = f(); };
}
+
+namespace template_valid {
+// Valid, we shouldn't build a CXXDefaultInitExpr until A's ctor definition.
+struct A {
+ A();
+ template <typename T>
+ struct B { int m1 = sizeof(A) + sizeof(T); };
+ B<int> m2;
+};
+A::A() {}
+}
+
+namespace template_default_ctor {
+struct A {
+ template <typename T>
+ struct B {
+ int m1 = 0; // expected-error {{cannot default initialize non-static data member 'm1' before the end of the outermost containing class 'A'}}
+ };
+ // expected-note@+1 {{implicit default constructor for 'template_default_ctor::A::B<int>' first required here}}
+ enum { NOE = noexcept(B<int>()) };
+};
+}
+
+namespace default_ctor {
+struct A {
+ struct B {
+ int m1 = 0; // expected-error {{cannot default initialize non-static data member 'm1' before the end of the outermost containing class 'A'}}
+ };
+ // expected-note@+1 {{implicit default constructor for 'default_ctor::A::B' first required here}}
+ enum { NOE = noexcept(B()) };
+};
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
