Tyker created this revision.
Tyker added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
this patch improves diagnostic for invalid constexpr defaulted special members
by adding notes explaining why the special member cannot be constexpr.
example
input:
01 struct B {};
02
03 struct C : virtual B {
04 };
05
06 struct D {
07 C c;
08 };
09
10 struct A : D {
11 constexpr A() = default;
12 };
13
14 union U {
15 A a;
16 int b;
17 constexpr U() = default;
18 };
19
20 struct E {
21 ~E() {}
22 };
23
24 struct F {
25 E e;
26 constexpr F& operator=(const F&) =default;
27 };
output with patch:
test.cpp:11:3: error: defaulted definition of default constructor is not
constexpr because:
constexpr A() = default;
^
test.cpp:10:12: note: base class 'D' of 'A' has a non-constexpr implicit
default constructor
struct A : D {
^
test.cpp:7:5: note: non-static data member 'c' of 'D' has a non-constexpr
implicit default constructor
C c;
^
test.cpp:3:12: note: 'C' inherits virtually from 'B'
struct C : virtual B {
^
test.cpp:17:3: error: defaulted definition of default constructor is not
constexpr because:
constexpr U() = default;
^
note: unions require exactly one non-static data member initializer to have a
constexpr default constructor
test.cpp:26:3: error: defaulted definition of copy assignment operator is not
constexpr because:
constexpr F& operator=(const F&) =default;
^
note: 'F' is not a literal type
3 errors generated.
I didn't adapt exitsing tests yet because the diagnostics emitted are likely be
adapted during review.
Repository:
rC Clang
https://reviews.llvm.org/D63134
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6366,19 +6366,26 @@
Sema::CXXSpecialMember CSM, unsigned Quals,
bool ConstRHS,
CXXConstructorDecl *InheritedCtor = nullptr,
- Sema::InheritedConstructorInfo *Inherited = nullptr) {
+ Sema::InheritedConstructorInfo *Inherited = nullptr,
+ SourceLocation* SMLoc = nullptr) {
// If we're inheriting a constructor, see if we need to call it for this base
// class.
if (InheritedCtor) {
assert(CSM == Sema::CXXDefaultConstructor);
auto BaseCtor =
Inherited->findConstructorForBase(ClassDecl, InheritedCtor).first;
- if (BaseCtor)
+ if (BaseCtor) {
+ if (SMLoc && !BaseCtor->isImplicit())
+ *SMLoc = BaseCtor->getLocation();
return BaseCtor->isConstexpr();
+ }
}
- if (CSM == Sema::CXXDefaultConstructor)
- return ClassDecl->hasConstexprDefaultConstructor();
+ if (CSM == Sema::CXXDefaultConstructor) {
+ bool IsConstexprCtor = ClassDecl->hasConstexprDefaultConstructor();
+ if (IsConstexprCtor || !SMLoc)
+ return IsConstexprCtor;
+ }
Sema::SpecialMemberOverloadResult SMOR =
lookupCallFromSpecialMember(S, ClassDecl, CSM, Quals, ConstRHS);
@@ -6386,21 +6393,96 @@
// A constructor we wouldn't select can't be "involved in initializing"
// anything.
return true;
+ if (SMLoc && !SMOR.getMethod()->isImplicit())
+ *SMLoc = SMOR.getMethod()->getLocation();
return SMOR.getMethod()->isConstexpr();
}
/// Determine whether the specified special member function would be constexpr
/// if it were implicitly defined.
static bool defaultedSpecialMemberIsConstexpr(
- Sema &S, CXXRecordDecl *ClassDecl, Sema::CXXSpecialMember CSM,
- bool ConstArg, CXXConstructorDecl *InheritedCtor = nullptr,
- Sema::InheritedConstructorInfo *Inherited = nullptr) {
+ Sema &S, const CXXRecordDecl *ClassDecl, Sema::CXXSpecialMember CSM,
+ bool ConstArg,
+ CXXConstructorDecl *InheritedCtor = nullptr,
+ Sema::InheritedConstructorInfo *Inherited = nullptr,
+ SmallVectorImpl<PartialDiagnosticAt>* DiagNotes = nullptr) {
if (!S.getLangOpts().CPlusPlus11)
return false;
+ SourceLocation SMLoc;
+ SourceLocation DiagLoc;
+ enum class FailKind {
+ Subobject,
+ Union,
+ VirtualBase,
+ NonLiteral,
+ };
+ /// This lambda will gathering all information and add notes for diagnostics.
+ auto Fail = [&](FailKind FailureKind, const Decl *FaillingDecl = nullptr) {
+ if (DiagNotes) {
+ switch (FailureKind) {
+ case FailKind::Subobject: {
+ PartialDiagnostic PDiag(diag::note_constexpr_defaulted_special_member, S.Context.getDiagAllocator());
+ auto *Field = dyn_cast<FieldDecl>(FaillingDecl);
+ auto *Record = dyn_cast<CXXRecordDecl>(FaillingDecl);
+ SourceLocation Loc = FaillingDecl->getLocation();
+ if (!Record)
+ Record = Field->getType()->getAsCXXRecordDecl();
+ if (DiagLoc.isValid())
+ Loc = DiagLoc;
+ PDiag << CSM << isa<CXXRecordDecl>(FaillingDecl);
+ if (Field)
+ PDiag << Field;
+ else
+ PDiag << Record;
+ PDiag << ClassDecl << SMLoc.isInvalid()
+ << (Field ? Field->getDeclName().isEmpty() : false);
+ DiagNotes->emplace_back(Loc, PDiag);
+ if (SMLoc.isValid()) {
+ DiagNotes->emplace_back(
+ SMLoc, PartialDiagnostic(diag::note_special_member_declared_here,
+ S.Context.getDiagAllocator())
+ << CSM);
+ return false;
+ }
+ if (!Record)
+ return false;
+ defaultedSpecialMemberIsConstexpr(S, Record, CSM, ConstArg, nullptr,
+ nullptr, DiagNotes);
+ return false;
+ }
+ case FailKind::VirtualBase: {
+ PartialDiagnostic PDiag(diag::note_constexpr_defaulted_virtual_base,
+ S.Context.getDiagAllocator());
+ const CXXBaseSpecifier& Base = *ClassDecl->bases_begin();
+ PDiag << ClassDecl << Base.getType();
+ DiagNotes->emplace_back(Base.getBeginLoc(), PDiag);
+ return false;
+ }
+ case FailKind::Union:
+ DiagNotes->emplace_back(
+ SourceLocation(),
+ PartialDiagnostic(diag::note_constexpr_defaulted_union,
+ S.Context.getDiagAllocator())
+ << CSM);
+ return false;
+ case FailKind::NonLiteral:
+ // FIXME: Explain why it is not literal.
+ DiagNotes->emplace_back(
+ SourceLocation(),
+ PartialDiagnostic(diag::note_constexpr_defaulted_non_literal,
+ S.Context.getDiagAllocator())
+ << ClassDecl);
+ return false;
+ }
+ }
+ return false;
+ };
+
// C++11 [dcl.constexpr]p4:
// In the definition of a constexpr constructor [...]
bool Ctor = true;
+ bool IsCtorConstexpr = false;
switch (CSM) {
case Sema::CXXDefaultConstructor:
if (Inherited)
@@ -6411,7 +6493,10 @@
//
// This is important for performance; we need to know whether the default
// constructor is constexpr to determine whether the type is a literal type.
- return ClassDecl->defaultedDefaultConstructorIsConstexpr();
+ IsCtorConstexpr = ClassDecl->defaultedDefaultConstructorIsConstexpr();
+ if (DiagNotes && !IsCtorConstexpr)
+ break;
+ return IsCtorConstexpr;
case Sema::CXXCopyConstructor:
case Sema::CXXMoveConstructor:
@@ -6438,20 +6523,23 @@
// If we squint, this is guaranteed, since exactly one non-static data member
// will be initialized (if the constructor isn't deleted), we just don't know
// which one.
- if (Ctor && ClassDecl->isUnion())
- return CSM == Sema::CXXDefaultConstructor
- ? ClassDecl->hasInClassInitializer() ||
- !ClassDecl->hasVariantMembers()
- : true;
+ if (Ctor && ClassDecl->isUnion()) {
+ if (CSM == Sema::CXXDefaultConstructor &&
+ !(ClassDecl->hasInClassInitializer() ||
+ !ClassDecl->hasVariantMembers()))
+ return Fail(FailKind::Union);
+ else
+ return true;
+ }
// -- the class shall not have any virtual base classes;
if (Ctor && ClassDecl->getNumVBases())
- return false;
+ return Fail(FailKind::VirtualBase);
// C++1y [class.copy]p26:
// -- [the class] is a literal type, and
if (!Ctor && !ClassDecl->isLiteral())
- return false;
+ return Fail(FailKind::NonLiteral);
// -- every constructor involved in initializing [...] base class
// sub-objects shall be a constexpr constructor;
@@ -6461,10 +6549,11 @@
const RecordType *BaseType = B.getType()->getAs<RecordType>();
if (!BaseType) continue;
+ DiagLoc = B.getBeginLoc();
CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(BaseType->getDecl());
if (!specialMemberIsConstexpr(S, BaseClassDecl, CSM, 0, ConstArg,
- InheritedCtor, Inherited))
- return false;
+ InheritedCtor, Inherited, DiagNotes ? &SMLoc : nullptr))
+ return Fail(FailKind::Subobject, BaseClassDecl);
}
// -- every constructor involved in initializing non-static data members
@@ -6484,10 +6573,11 @@
CXXRecordDecl *FieldRecDecl = cast<CXXRecordDecl>(RecordTy->getDecl());
if (!specialMemberIsConstexpr(S, FieldRecDecl, CSM,
BaseType.getCVRQualifiers(),
- ConstArg && !F->isMutable()))
- return false;
+ ConstArg && !F->isMutable(),
+ nullptr, nullptr, DiagNotes ? &SMLoc : nullptr))
+ return Fail(FailKind::Subobject, F);
} else if (CSM == Sema::CXXDefaultConstructor) {
- return false;
+ return Fail(FailKind::Subobject, F);
}
}
@@ -6682,14 +6772,20 @@
// destructors in C++1y), this is checked elsewhere.
//
// FIXME: This should not apply if the member is deleted.
- bool Constexpr = defaultedSpecialMemberIsConstexpr(*this, RD, CSM,
- HasConstParam);
- if ((getLangOpts().CPlusPlus14 ? !isa<CXXDestructorDecl>(MD)
- : isa<CXXConstructorDecl>(MD)) &&
- MD->isConstexpr() && !Constexpr &&
- MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
- Diag(MD->getBeginLoc(), diag::err_incorrect_defaulted_constexpr) << CSM;
- // FIXME: Explain why the special member can't be constexpr.
+ bool InvalidIfNotConstexpr =
+ ((getLangOpts().CPlusPlus14 ? !isa<CXXDestructorDecl>(MD)
+ : isa<CXXConstructorDecl>(MD)) &&
+ MD->isConstexpr() &&
+ MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate);
+ llvm::SmallVector<PartialDiagnosticAt, 8> DiagNotes;
+ bool Constexpr = defaultedSpecialMemberIsConstexpr(
+ *this, RD, CSM, HasConstParam, /*InheritedCtor=*/nullptr,
+ /*Inherited=*/nullptr, InvalidIfNotConstexpr ? &DiagNotes : nullptr);
+ if (InvalidIfNotConstexpr && !Constexpr) {
+ Diag(MD->getBeginLoc(), diag::err_incorrect_defaulted_constexpr)
+ << CSM << (DiagNotes.size() != 0);
+ for (auto Note : DiagNotes)
+ Diag(Note.first, Note.second);
HadError = true;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7846,7 +7846,7 @@
"lvalue reference type">;
def err_incorrect_defaulted_constexpr : Error<
"defaulted definition of %sub{select_special_member_kind}0 "
- "is not constexpr">;
+ "is not constexpr%select{| because:}1">;
def warn_defaulted_method_deleted : Warning<
"explicitly defaulted %sub{select_special_member_kind}0 is implicitly "
"deleted">, InGroup<DiagGroup<"defaulted-function-deleted">>;
@@ -7866,7 +7866,19 @@
def note_vbase_moved_here : Note<
"%select{%1 is a virtual base class of base class %2 declared here|"
"virtual base class %1 declared here}0">;
-
+def note_special_member_declared_here : Note<
+ "%sub{select_special_member_kind}0 declared here">;
+def note_constexpr_defaulted_special_member : Note<
+ "%select{|anonymous }5%select{non-static data member|base class}1"
+ "%select{ %2|}5 of %3 has a non-constexpr %select{|implicit }4"
+ "%sub{select_special_member_kind}0">;
+def note_constexpr_defaulted_virtual_base : Note<
+ "%0 inherits virtually from %1">;
+def note_constexpr_defaulted_union : Note<
+ "unions require exactly one non-static data member initializer"
+ " to have a constexpr %sub{select_special_member_kind}0">;
+def note_constexpr_defaulted_non_literal : Note<
+ "%0 is not a literal type">;
def ext_implicit_exception_spec_mismatch : ExtWarn<
"function previously declared with an %select{explicit|implicit}0 exception "
"specification redeclared with an %select{implicit|explicit}0 exception "
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
