From e16da07474c376fbbeda2d4238cf35e2bd664d5a Mon Sep 17 00:00:00 2001
From: Sean Hunt <scshunt@csclub.uwaterloo.ca>
Date: Mon, 10 Oct 2011 06:18:57 +0000
Subject: [PATCH] Begin work consolidating ShouldDelete* functions.
Begin with just default constructors. One note is that as a side effect
of this, a conformance test was removed on the basis that this is almost
certainly a defect as with most of union initialization. As it is, clang
does not implement union initialization close to the standard as it's
quite broken as written. I hope to write a paper addressing the issues
eventually.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@141528 91177308-0d34-0410-b5e6-96231b3b80d8
---
include/clang/Sema/Sema.h | 6 +-
lib/Sema/SemaDeclCXX.cpp | 220 +++++++++++++++-----------
test/CXX/special/class.ctor/p5-0x.cpp | 4 -
3 files changed, 134 insertions(+), 96 deletions(-)
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 357b7696ae3..f462d5bc9df 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -2830,9 +2830,9 @@ public:
ImplicitExceptionSpecification
ComputeDefaultedDtorExceptionSpec(CXXRecordDecl *ClassDecl);
- /// \brief Determine if a defaulted default constructor ought to be
- /// deleted.
- bool ShouldDeleteDefaultConstructor(CXXConstructorDecl *CD);
+ /// \brief Determine if a special member function should have a deleted
+ /// definition when it is defaulted.
+ bool ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM);
/// \brief Determine if a defaulted copy constructor ought to be
/// deleted.
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index b16cbb80c40..cce6a532fdd 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -3742,7 +3742,7 @@ void Sema::CheckExplicitlyDefaultedDefaultConstructor(CXXConstructorDecl *CD) {
return;
}
- if (ShouldDeleteDefaultConstructor(CD)) {
+ if (ShouldDeleteSpecialMember(CD, CXXDefaultConstructor)) {
if (First) {
CD->setDeletedAsWritten();
} else {
@@ -4096,30 +4096,44 @@ void Sema::CheckExplicitlyDefaultedDestructor(CXXDestructorDecl *DD) {
}
}
-bool Sema::ShouldDeleteDefaultConstructor(CXXConstructorDecl *CD) {
- CXXRecordDecl *RD = CD->getParent();
+/// This function implements the following C++0x paragraphs:
+/// - [class.ctor]/5
+bool Sema::ShouldDeleteSpecialMember(CXXMethodDecl *MD, CXXSpecialMember CSM) {
+ assert(!MD->isInvalidDecl());
+ CXXRecordDecl *RD = MD->getParent();
assert(!RD->isDependentType() && "do deletion after instantiation");
if (!LangOpts.CPlusPlus0x || RD->isInvalidDecl())
return false;
- SourceLocation Loc = CD->getLocation();
+ bool IsUnion = RD->isUnion();
+ bool IsConstructor = false;
+ bool IsAssignment = false;
+ bool IsMove = false;
+
+ bool ConstArg = false;
+
+ switch (CSM) {
+ case CXXDefaultConstructor:
+ IsConstructor = true;
+ break;
+ default:
+ llvm_unreachable("function only currently implemented for default ctors");
+ }
+
+ SourceLocation Loc = MD->getLocation();
// Do access control from the constructor
- ContextRAII CtorContext(*this, CD);
+ ContextRAII MethodContext(*this, MD);
- bool Union = RD->isUnion();
bool AllConst = true;
// We do this because we should never actually use an anonymous
// union's constructor.
- if (Union && RD->isAnonymousStructOrUnion())
+ if (IsUnion && RD->isAnonymousStructOrUnion())
return false;
// FIXME: We should put some diagnostic logic right into this function.
- // C++0x [class.ctor]/5
- // A defaulted default constructor for class X is defined as deleted if:
-
for (CXXRecordDecl::base_class_iterator BI = RD->bases_begin(),
BE = RD->bases_end();
BI != BE; ++BI) {
@@ -4130,26 +4144,33 @@ bool Sema::ShouldDeleteDefaultConstructor(CXXConstructorDecl *CD) {
CXXRecordDecl *BaseDecl = BI->getType()->getAsCXXRecordDecl();
assert(BaseDecl && "base isn't a CXXRecordDecl");
- // -- any [direct base class] has a type with a destructor that is
- // deleted or inaccessible from the defaulted default constructor
- CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl);
- if (BaseDtor->isDeleted())
- return true;
- if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
- AR_accessible)
- return true;
-
- // -- any [direct base class either] has no default constructor or
- // overload resolution as applied to [its] default constructor
- // results in an ambiguity or in a function that is deleted or
- // inaccessible from the defaulted default constructor
- CXXConstructorDecl *BaseDefault = LookupDefaultConstructor(BaseDecl);
- if (!BaseDefault || BaseDefault->isDeleted())
- return true;
+ // Unless we have an assignment operator, the base's destructor must
+ // be accessible and not deleted.
+ if (!IsAssignment) {
+ CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl);
+ if (BaseDtor->isDeleted())
+ return true;
+ if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
+ AR_accessible)
+ return true;
+ }
- if (CheckConstructorAccess(Loc, BaseDefault, BaseDefault->getAccess(),
- PDiag()) != AR_accessible)
- return true;
+ // Finding the corresponding member in the base should lead to a
+ // unique, accessible, non-deleted function.
+ if (CSM != CXXDestructor) {
+ SpecialMemberOverloadResult *SMOR =
+ LookupSpecialMember(BaseDecl, CSM, ConstArg, false, IsMove, false,
+ false);
+ if (!SMOR->hasSuccess())
+ return true;
+ CXXMethodDecl *BaseMember = SMOR->getMethod();
+ if (IsConstructor) {
+ CXXConstructorDecl *BaseCtor = cast<CXXConstructorDecl>(BaseMember);
+ if (CheckConstructorAccess(Loc, BaseCtor, BaseCtor->getAccess(),
+ PDiag()) != AR_accessible)
+ return true;
+ }
+ }
}
for (CXXRecordDecl::base_class_iterator BI = RD->vbases_begin(),
@@ -4158,26 +4179,33 @@ bool Sema::ShouldDeleteDefaultConstructor(CXXConstructorDecl *CD) {
CXXRecordDecl *BaseDecl = BI->getType()->getAsCXXRecordDecl();
assert(BaseDecl && "base isn't a CXXRecordDecl");
- // -- any [virtual base class] has a type with a destructor that is
- // delete or inaccessible from the defaulted default constructor
- CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl);
- if (BaseDtor->isDeleted())
- return true;
- if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
- AR_accessible)
- return true;
-
- // -- any [virtual base class either] has no default constructor or
- // overload resolution as applied to [its] default constructor
- // results in an ambiguity or in a function that is deleted or
- // inaccessible from the defaulted default constructor
- CXXConstructorDecl *BaseDefault = LookupDefaultConstructor(BaseDecl);
- if (!BaseDefault || BaseDefault->isDeleted())
- return true;
+ // Unless we have an assignment operator, the base's destructor must
+ // be accessible and not deleted.
+ if (!IsAssignment) {
+ CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl);
+ if (BaseDtor->isDeleted())
+ return true;
+ if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
+ AR_accessible)
+ return true;
+ }
- if (CheckConstructorAccess(Loc, BaseDefault, BaseDefault->getAccess(),
- PDiag()) != AR_accessible)
- return true;
+ // Finding the corresponding member in the base should lead to a
+ // unique, accessible, non-deleted function.
+ if (CSM != CXXDestructor) {
+ SpecialMemberOverloadResult *SMOR =
+ LookupSpecialMember(BaseDecl, CSM, ConstArg, false, IsMove, false,
+ false);
+ if (!SMOR->hasSuccess())
+ return true;
+ CXXMethodDecl *BaseMember = SMOR->getMethod();
+ if (IsConstructor) {
+ CXXConstructorDecl *BaseCtor = cast<CXXConstructorDecl>(BaseMember);
+ if (CheckConstructorAccess(Loc, BaseCtor, BaseCtor->getAccess(),
+ PDiag()) != AR_accessible)
+ return true;
+ }
+ }
}
for (CXXRecordDecl::field_iterator FI = RD->field_begin(),
@@ -4189,38 +4217,38 @@ bool Sema::ShouldDeleteDefaultConstructor(CXXConstructorDecl *CD) {
QualType FieldType = Context.getBaseElementType(FI->getType());
CXXRecordDecl *FieldRecord = FieldType->getAsCXXRecordDecl();
- // -- any non-static data member with no brace-or-equal-initializer is of
- // reference type
- if (FieldType->isReferenceType() && !FI->hasInClassInitializer())
- return true;
+ // For a default constructor, all references must be initialized in-class
+ // and, if a union, it must have a non-const member.
+ if (CSM == CXXDefaultConstructor) {
+ if (FieldType->isReferenceType() && !FI->hasInClassInitializer())
+ return true;
- // -- X is a union and all its variant members are of const-qualified type
- // (or array thereof)
- if (Union && !FieldType.isConstQualified())
- AllConst = false;
+ if (IsUnion && !FieldType.isConstQualified())
+ AllConst = false;
+ }
if (FieldRecord) {
- // -- X is a union-like class that has a variant member with a non-trivial
- // default constructor
- if (Union && !FieldRecord->hasTrivialDefaultConstructor())
- return true;
-
- CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord);
- if (FieldDtor->isDeleted())
- return true;
- if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) !=
- AR_accessible)
- return true;
+ // Unless we're doing assignment, the field's destructor must be
+ // accessible and not deleted.
+ if (!IsAssignment) {
+ CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord);
+ if (FieldDtor->isDeleted())
+ return true;
+ if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) !=
+ AR_accessible)
+ return true;
+ }
- // -- any non-variant non-static data member of const-qualified type (or
- // array thereof) with no brace-or-equal-initializer does not have a
- // user-provided default constructor
- if (FieldType.isConstQualified() &&
+ // For a default constructor, a const member must have a user-provided
+ // default constructor or else be explicitly initialized.
+ if (CSM == CXXDefaultConstructor && FieldType.isConstQualified() &&
!FI->hasInClassInitializer() &&
!FieldRecord->hasUserProvidedDefaultConstructor())
return true;
- if (!Union && FieldRecord->isUnion() &&
+ // For a default constructor, additional restrictions exist on the
+ // variant members.
+ if (CSM == CXXDefaultConstructor && !IsUnion && FieldRecord->isUnion() &&
FieldRecord->isAnonymousStructOrUnion()) {
// We're okay to reuse AllConst here since we only care about the
// value otherwise if we're in a union.
@@ -4249,29 +4277,43 @@ bool Sema::ShouldDeleteDefaultConstructor(CXXConstructorDecl *CD) {
continue;
}
- // -- any non-static data member with no brace-or-equal-initializer has
- // class type M (or array thereof) and either M has no default
- // constructor or overload resolution as applied to M's default
- // constructor results in an ambiguity or in a function that is deleted
- // or inaccessible from the defaulted default constructor.
- if (!FI->hasInClassInitializer()) {
- CXXConstructorDecl *FieldDefault = LookupDefaultConstructor(FieldRecord);
- if (!FieldDefault || FieldDefault->isDeleted())
+ // Check that the corresponding member of the field is accessible,
+ // unique, and non-deleted. We don't do this if it has an explicit
+ // initialization when default-constructing.
+ if (CSM != CXXDestructor &&
+ (CSM != CXXDefaultConstructor || !FI->hasInClassInitializer())) {
+ SpecialMemberOverloadResult *SMOR =
+ LookupSpecialMember(FieldRecord, CSM, ConstArg, false, IsMove, false,
+ false);
+ if (!SMOR->hasSuccess())
return true;
- if (CheckConstructorAccess(Loc, FieldDefault, FieldDefault->getAccess(),
- PDiag()) != AR_accessible)
+
+ CXXMethodDecl *FieldMember = SMOR->getMethod();
+ if (IsConstructor) {
+ CXXConstructorDecl *FieldCtor = cast<CXXConstructorDecl>(FieldMember);
+ if (CheckConstructorAccess(Loc, FieldCtor, FieldCtor->getAccess(),
+ PDiag()) != AR_accessible)
+ return true;
+ }
+
+ // We need the corresponding member of a union to be trivial so that
+ // we can safely copy them all simultaneously.
+ // FIXME: Note that performing the check here (where we rely on the lack
+ // of an in-class initializer) is technically ill-formed. However, this
+ // seems most obviously to be a bug in the standard.
+ if (IsUnion && !FieldMember->isTrivial())
return true;
}
- } else if (!Union && FieldType.isConstQualified() &&
- !FI->hasInClassInitializer()) {
- // -- any non-variant non-static data member of const-qualified type (or
- // array thereof) with no brace-or-equal-initializer does not have a
- // user-provided default constructor
+ } else if (CSM == CXXDefaultConstructor && !IsUnion &&
+ FieldType.isConstQualified() && !FI->hasInClassInitializer()) {
+ // We can't initialize a const member of non-class type to any value.
return true;
}
}
- if (Union && AllConst)
+ // We can't have all const members in a union when default-constructing,
+ // or else they're all nonsensical garbage values that can't be changed.
+ if (CSM == CXXDefaultConstructor && IsUnion && AllConst)
return true;
return false;
@@ -6979,7 +7021,7 @@ CXXConstructorDecl *Sema::DeclareImplicitDefaultConstructor(
PushOnScopeChains(DefaultCon, S, false);
ClassDecl->addDecl(DefaultCon);
- if (ShouldDeleteDefaultConstructor(DefaultCon))
+ if (ShouldDeleteSpecialMember(DefaultCon, CXXDefaultConstructor))
DefaultCon->setDeletedAsWritten();
return DefaultCon;
diff --git a/test/CXX/special/class.ctor/p5-0x.cpp b/test/CXX/special/class.ctor/p5-0x.cpp
index 2123d166235..c1b00e45d60 100644
--- a/test/CXX/special/class.ctor/p5-0x.cpp
+++ b/test/CXX/special/class.ctor/p5-0x.cpp
@@ -23,10 +23,6 @@ int n;
// default constructor,
union Deleted1a { UserProvidedDefCtor u; }; // expected-note {{deleted here}}
Deleted1a d1a; // expected-error {{deleted constructor}}
-// FIXME: treating this as having a deleted default constructor is probably a
-// bug in the standard.
-union Deleted1b { UserProvidedDefCtor u = UserProvidedDefCtor(); }; // expected-note {{deleted here}}
-Deleted1b d1b; // expected-error {{deleted constructor}}
union NotDeleted1a { DefaultedDefCtor1 nu; };
NotDeleted1a nd1a;
// FIXME: clang implements the pre-FDIS rule, under which DefaultedDefCtor2's
--
GitLab