From 49634cf3b9b0c3da2aedc3bdefbf331bce167239 Mon Sep 17 00:00:00 2001 From: Sean Hunt Date: Fri, 13 May 2011 06:10:58 +0000 Subject: [PATCH] Defaulting copy constructors now works reasonably well. One more special member to go git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@131287 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 8 + include/clang/Sema/Sema.h | 18 +- lib/Sema/SemaDeclCXX.cpp | 362 +++++++++++++++++++-- lib/Sema/SemaExpr.cpp | 5 +- 4 files changed, 350 insertions(+), 43 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 8ef108d13b..e80b1f1649 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -3628,6 +3628,14 @@ def warn_explicit_conversion_functions : Warning< // C++0x defaulted functions def err_defaulted_default_ctor_params : Error< "an explicitly-defaulted default constructor must have no parameters">; +def err_defaulted_copy_ctor_params : Error< + "an explicitly-defaulted copy constructor must have exactly one parameter">; +def err_defaulted_copy_ctor_volatile_param : Error< + "the parameter for an explicitly-defaulted copy constructor may not be " + "volatile">; +def err_defaulted_copy_ctor_const_param : Error< + "the parameter for this explicitly-defaulted copy constructor is const, but " + "a member or base requires it to be non-const">; def err_incorrect_defaulted_exception_spec : Error< "exception specification of explicitly defaulted %select{default constructor|" "copy constructor|copy assignment operator|destructor}0 does not match the " diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 8f4bfac323..ca15b085b2 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -2531,7 +2531,8 @@ public: /// \brief Helper class that collects exception specifications for /// implicitly-declared special member functions. class ImplicitExceptionSpecification { - ASTContext &Context; + // Pointer to allow copying + ASTContext *Context; // We order exception specifications thus: // noexcept is the most restrictive, but is only used in C++0x. // throw() comes next. @@ -2549,7 +2550,7 @@ public: public: explicit ImplicitExceptionSpecification(ASTContext &Context) - : Context(Context), ComputedEST(EST_BasicNoexcept) { + : Context(&Context), ComputedEST(EST_BasicNoexcept) { if (!Context.getLangOptions().CPlusPlus0x) ComputedEST = EST_DynamicNone; } @@ -2584,6 +2585,11 @@ public: ImplicitExceptionSpecification ComputeDefaultedDefaultCtorExceptionSpec(CXXRecordDecl *ClassDecl); + /// \brief Determine what sort of exception specification a defaulted + /// constructor of a class will have. + std::pair + ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl); + /// \brief Determine what sort of exception specification a defaulted /// destructor of a class will have. ImplicitExceptionSpecification @@ -2593,6 +2599,10 @@ public: /// deleted. bool ShouldDeleteDefaultConstructor(CXXConstructorDecl *CD); + /// \brief Determine if a defaulted copy constructor ought to be + /// deleted. + bool ShouldDeleteCopyConstructor(CXXConstructorDecl *CD); + /// \brief Determine if a defaulted destructor ought to be deleted. bool ShouldDeleteDestructor(CXXDestructorDecl *DD); @@ -2643,8 +2653,7 @@ public: /// DefineImplicitCopyConstructor - Checks for feasibility of /// defining this constructor as the copy constructor. void DefineImplicitCopyConstructor(SourceLocation CurrentLocation, - CXXConstructorDecl *Constructor, - unsigned TypeQuals); + CXXConstructorDecl *Constructor); /// \brief Declare the implicit copy assignment operator for the given class. /// @@ -3271,6 +3280,7 @@ public: void CheckExplicitlyDefaultedMethods(CXXRecordDecl *Record); void CheckExplicitlyDefaultedDefaultConstructor(CXXConstructorDecl *Ctor); + void CheckExplicitlyDefaultedCopyConstructor(CXXConstructorDecl *Ctor); void CheckExplicitlyDefaultedDestructor(CXXDestructorDecl *Dtor); //===--------------------------------------------------------------------===// diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index b78f01c94d..77d20aec83 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -112,6 +112,7 @@ namespace { } void Sema::ImplicitExceptionSpecification::CalledDecl(CXXMethodDecl *Method) { + assert(Context && "ImplicitExceptionSpecification without an ASTContext"); // If we have an MSAny spec already, don't bother. if (!Method || ComputedEST == EST_MSAny) return; @@ -146,7 +147,7 @@ void Sema::ImplicitExceptionSpecification::CalledDecl(CXXMethodDecl *Method) { // Check out noexcept specs. if (EST == EST_ComputedNoexcept) { - FunctionProtoType::NoexceptResult NR = Proto->getNoexceptSpec(Context); + FunctionProtoType::NoexceptResult NR = Proto->getNoexceptSpec(*Context); assert(NR != FunctionProtoType::NR_NoNoexcept && "Must have noexcept result for EST_ComputedNoexcept."); assert(NR != FunctionProtoType::NR_Dependent && @@ -170,7 +171,7 @@ void Sema::ImplicitExceptionSpecification::CalledDecl(CXXMethodDecl *Method) { for (FunctionProtoType::exception_iterator E = Proto->exception_begin(), EEnd = Proto->exception_end(); E != EEnd; ++E) - if (ExceptionsSeen.insert(Context.getCanonicalType(*E))) + if (ExceptionsSeen.insert(Context->getCanonicalType(*E))) Exceptions.push_back(*E); } @@ -3013,8 +3014,11 @@ void Sema::CheckExplicitlyDefaultedMethods(CXXRecordDecl *Record) { break; case CXXCopyConstructor: + CheckExplicitlyDefaultedCopyConstructor(cast(*MI)); + break; + case CXXCopyAssignment: - // FIXME: Do copy and move constructors and assignment operators + // FIXME: Do copy assignment operators and moves break; default: @@ -3034,11 +3038,11 @@ void Sema::CheckExplicitlyDefaultedDefaultConstructor(CXXConstructorDecl *CD) { // that it would be deleted. (C++0x [decl.fct.def.default]) bool First = CD == CD->getCanonicalDecl(); + bool HadError = false; if (CD->getNumParams() != 0) { Diag(CD->getLocation(), diag::err_defaulted_default_ctor_params) << CD->getSourceRange(); - CD->setInvalidDecl(); - return; + HadError = true; } ImplicitExceptionSpecification Spec @@ -3055,8 +3059,7 @@ void Sema::CheckExplicitlyDefaultedDefaultConstructor(CXXConstructorDecl *CD) { PDiag(), ExceptionType, SourceLocation(), CtorType, CD->getLocation())) { - CD->setInvalidDecl(); - return; + HadError = true; } } else if (First) { // We set the declaration to have the computed exception spec here. @@ -3064,12 +3067,85 @@ void Sema::CheckExplicitlyDefaultedDefaultConstructor(CXXConstructorDecl *CD) { CD->setType(Context.getFunctionType(Context.VoidTy, 0, 0, EPI)); } + if (HadError) { + CD->setInvalidDecl(); + return; + } + if (ShouldDeleteDefaultConstructor(CD)) { - if (First) + if (First) { CD->setDeletedAsWritten(); - else + } else { Diag(CD->getLocation(), diag::err_out_of_line_default_deletes) << 0 /* default constructor */; + CD->setInvalidDecl(); + } + } +} + +void Sema::CheckExplicitlyDefaultedCopyConstructor(CXXConstructorDecl *CD) { + assert(CD->isExplicitlyDefaulted() && CD->isCopyConstructor()); + + // Whether this was the first-declared instance of the constructor. + bool First = CD == CD->getCanonicalDecl(); + + bool HadError = false; + if (CD->getNumParams() != 1) { + Diag(CD->getLocation(), diag::err_defaulted_copy_ctor_params) + << CD->getSourceRange(); + HadError = true; + } + + ImplicitExceptionSpecification Spec(Context); + bool Const; + llvm::tie(Spec, Const) = + ComputeDefaultedCopyCtorExceptionSpecAndConst(CD->getParent()); + + FunctionProtoType::ExtProtoInfo EPI = Spec.getEPI(); + const FunctionProtoType *CtorType = CD->getType()->getAs(), + *ExceptionType = Context.getFunctionType( + Context.VoidTy, 0, 0, EPI)->getAs(); + + // Check for parameter type matching. + // This is a copy ctor so we know it's a cv-qualified reference to T. + QualType ArgType = CtorType->getArgType(0); + if (ArgType->getPointeeType().isVolatileQualified()) { + Diag(CD->getLocation(), diag::err_defaulted_copy_ctor_volatile_param); + HadError = true; + } + if (ArgType->getPointeeType().isConstQualified() && !Const) { + Diag(CD->getLocation(), diag::err_defaulted_copy_ctor_const_param); + HadError = true; + } + + if (CtorType->hasExceptionSpec()) { + if (CheckEquivalentExceptionSpec( + PDiag(diag::err_incorrect_defaulted_exception_spec) + << 1 /* copy constructor */, + PDiag(), + ExceptionType, SourceLocation(), + CtorType, CD->getLocation())) { + HadError = true; + } + } else if (First) { + // We set the declaration to have the computed exception spec here. + // We duplicate the one parameter type. + CD->setType(Context.getFunctionType(Context.VoidTy, &ArgType, 1, EPI)); + } + + if (HadError) { + CD->setInvalidDecl(); + return; + } + + if (ShouldDeleteCopyConstructor(CD)) { + if (First) { + CD->setDeletedAsWritten(); + } else { + Diag(CD->getLocation(), diag::err_out_of_line_default_deletes) + << 1 /* copy constructor */; + CD->setInvalidDecl(); + } } } @@ -3103,11 +3179,13 @@ void Sema::CheckExplicitlyDefaultedDestructor(CXXDestructorDecl *DD) { } if (ShouldDeleteDestructor(DD)) { - if (First) + if (First) { DD->setDeletedAsWritten(); - else + } else { Diag(DD->getLocation(), diag::err_out_of_line_default_deletes) << 3 /* destructor */; + DD->setInvalidDecl(); + } } } @@ -3283,6 +3361,190 @@ bool Sema::ShouldDeleteDefaultConstructor(CXXConstructorDecl *CD) { return false; } +bool Sema::ShouldDeleteCopyConstructor(CXXConstructorDecl *CD) { + CXXRecordDecl *RD = CD->getParent(); + assert(!RD->isDependentType() && "do deletion after instantiation"); + if (!LangOpts.CPlusPlus0x) + return false; + + // Do access control from the constructor + ContextRAII CtorContext(*this, CD); + + bool Union = RD->isUnion(); + + bool ConstArg = CD->getParamDecl(0)->getType().isConstQualified(); + + // We do this because we should never actually use an anonymous + // union's constructor. + if (Union && RD->isAnonymousStructOrUnion()) + return false; + + // FIXME: We should put some diagnostic logic right into this function. + + // C++0x [class.copy]/11 + // A defaulted [copy] constructor for class X is defined as delete if X has: + + for (CXXRecordDecl::base_class_iterator BI = RD->bases_begin(), + BE = RD->bases_end(); + BI != BE; ++BI) { + // We'll handle this one later + if (BI->isVirtual()) + continue; + + QualType BaseType = BI->getType(); + CXXRecordDecl *BaseDecl = BaseType->getAsCXXRecordDecl(); + assert(BaseDecl && "base isn't a CXXRecordDecl"); + + // -- any [direct base class] of a type with a destructor that is deleted or + // inaccessible from the defaulted constructor + CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl); + if (BaseDtor->isDeleted()) + return true; + if (CheckDestructorAccess(SourceLocation(), BaseDtor, PDiag()) != + AR_accessible) + return true; + + // -- a [direct base class] B that cannot be [copied] because overload + // resolution, as applied to B's [copy] constructor, results in an + // ambiguity or a function that is deleted or inaccessible from the + // defaulted constructor + InitializedEntity BaseEntity = + InitializedEntity::InitializeBase(Context, BI, 0); + InitializationKind Kind = + InitializationKind::CreateDirect(SourceLocation(), SourceLocation(), + SourceLocation()); + + // Construct a fake expression to perform the copy overloading. + QualType ArgType = BaseType.getUnqualifiedType(); + if (ArgType->isReferenceType()) + ArgType = ArgType->getPointeeType(); + if (ConstArg) + ArgType.addConst(); + Expr *Arg = new (Context) OpaqueValueExpr(SourceLocation(), ArgType, + VK_LValue); + + InitializationSequence InitSeq(*this, BaseEntity, Kind, &Arg, 1); + + if (InitSeq.getKind() == InitializationSequence::FailedSequence) + return true; + } + + for (CXXRecordDecl::base_class_iterator BI = RD->vbases_begin(), + BE = RD->vbases_end(); + BI != BE; ++BI) { + QualType BaseType = BI->getType(); + CXXRecordDecl *BaseDecl = BaseType->getAsCXXRecordDecl(); + assert(BaseDecl && "base isn't a CXXRecordDecl"); + + // -- any [direct base class] of a type with a destructor that is deleted or + // inaccessible from the defaulted constructor + CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl); + if (BaseDtor->isDeleted()) + return true; + if (CheckDestructorAccess(SourceLocation(), BaseDtor, PDiag()) != + AR_accessible) + return true; + + // -- a [virtual base class] B that cannot be [copied] because overload + // resolution, as applied to B's [copy] constructor, results in an + // ambiguity or a function that is deleted or inaccessible from the + // defaulted constructor + InitializedEntity BaseEntity = + InitializedEntity::InitializeBase(Context, BI, BI); + InitializationKind Kind = + InitializationKind::CreateDirect(SourceLocation(), SourceLocation(), + SourceLocation()); + + // Construct a fake expression to perform the copy overloading. + QualType ArgType = BaseType.getUnqualifiedType(); + if (ArgType->isReferenceType()) + ArgType = ArgType->getPointeeType(); + if (ConstArg) + ArgType.addConst(); + Expr *Arg = new (Context) OpaqueValueExpr(SourceLocation(), ArgType, + VK_LValue); + + InitializationSequence InitSeq(*this, BaseEntity, Kind, &Arg, 1); + + if (InitSeq.getKind() == InitializationSequence::FailedSequence) + return true; + } + + for (CXXRecordDecl::field_iterator FI = RD->field_begin(), + FE = RD->field_end(); + FI != FE; ++FI) { + QualType FieldType = Context.getBaseElementType(FI->getType()); + + // -- for a copy constructor, a non-static data member of rvalue reference + // type + if (FieldType->isRValueReferenceType()) + return true; + + CXXRecordDecl *FieldRecord = FieldType->getAsCXXRecordDecl(); + + if (FieldRecord) { + // This is an anonymous union + if (FieldRecord->isUnion() && FieldRecord->isAnonymousStructOrUnion()) { + // Anonymous unions inside unions do not variant members create + if (!Union) { + for (CXXRecordDecl::field_iterator UI = FieldRecord->field_begin(), + UE = FieldRecord->field_end(); + UI != UE; ++UI) { + QualType UnionFieldType = Context.getBaseElementType(UI->getType()); + CXXRecordDecl *UnionFieldRecord = + UnionFieldType->getAsCXXRecordDecl(); + + // -- a variant member with a non-trivial [copy] constructor and X + // is a union-like class + if (UnionFieldRecord && + !UnionFieldRecord->hasTrivialCopyConstructor()) + return true; + } + } + + // Don't try to initalize an anonymous union + continue; + } else { + // -- a variant member with a non-trivial [copy] constructor and X is a + // union-like class + if (Union && !FieldRecord->hasTrivialCopyConstructor()) + return true; + + // -- any [non-static data member] of a type with a destructor that is + // deleted or inaccessible from the defaulted constructor + CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord); + if (FieldDtor->isDeleted()) + return true; + if (CheckDestructorAccess(SourceLocation(), FieldDtor, PDiag()) != + AR_accessible) + return true; + } + } + + InitializedEntity MemberEntity = + InitializedEntity::InitializeMember(*FI, 0); + InitializationKind Kind = + InitializationKind::CreateDirect(SourceLocation(), SourceLocation(), + SourceLocation()); + + // Construct a fake expression to perform the copy overloading. + QualType ArgType = FieldType; + if (ArgType->isReferenceType()) + ArgType = ArgType->getPointeeType(); + if (ConstArg) + ArgType.addConst(); + Expr *Arg = new (Context) OpaqueValueExpr(SourceLocation(), ArgType, + VK_LValue); + + InitializationSequence InitSeq(*this, MemberEntity, Kind, &Arg, 1); + + if (InitSeq.getKind() == InitializationSequence::FailedSequence) + return true; + } + + return false; +} + bool Sema::ShouldDeleteDestructor(CXXDestructorDecl *DD) { CXXRecordDecl *RD = DD->getParent(); assert(!RD->isDependentType() && "do deletion after instantiation"); @@ -3294,6 +3556,11 @@ bool Sema::ShouldDeleteDestructor(CXXDestructorDecl *DD) { bool Union = RD->isUnion(); + // We do this because we should never actually use an anonymous + // union's destructor. + if (Union && RD->isAnonymousStructOrUnion()) + return false; + // C++0x [class.dtor]p5 // A defaulted destructor for a class X is defined as deleted if: for (CXXRecordDecl::base_class_iterator BI = RD->bases_begin(), @@ -6418,12 +6685,8 @@ void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation, } } -CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor( - CXXRecordDecl *ClassDecl) { - // C++ [class.copy]p4: - // If the class definition does not explicitly declare a copy - // constructor, one is declared implicitly. - +std::pair +Sema::ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl) { // C++ [class.copy]p5: // The implicitly-declared copy constructor for a class X will // have the form @@ -6485,17 +6748,11 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor( = FieldClassDecl->hasConstCopyConstructor(Context); } } - // Otherwise, the implicitly declared copy constructor will have // the form // // X::X(X&) - QualType ClassType = Context.getTypeDeclType(ClassDecl); - QualType ArgType = ClassType; - if (HasConstCopyConstructor) - ArgType = ArgType.withConst(); - ArgType = Context.getLValueReferenceType(ArgType); - + // C++ [except.spec]p14: // An implicitly declared special member function (Clause 12) shall have an // exception-specification. [...] @@ -6548,17 +6805,36 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor( } } - // An implicitly-declared copy constructor is an inline public - // member of its class. - FunctionProtoType::ExtProtoInfo EPI; - EPI.ExceptionSpecType = ExceptSpec.getExceptionSpecType(); - EPI.NumExceptions = ExceptSpec.size(); - EPI.Exceptions = ExceptSpec.data(); + return std::make_pair(ExceptSpec, HasConstCopyConstructor); +} + +CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor( + CXXRecordDecl *ClassDecl) { + // C++ [class.copy]p4: + // If the class definition does not explicitly declare a copy + // constructor, one is declared implicitly. + + ImplicitExceptionSpecification Spec(Context); + bool Const; + llvm::tie(Spec, Const) = + ComputeDefaultedCopyCtorExceptionSpecAndConst(ClassDecl); + + QualType ClassType = Context.getTypeDeclType(ClassDecl); + QualType ArgType = ClassType; + if (Const) + ArgType = ArgType.withConst(); + ArgType = Context.getLValueReferenceType(ArgType); + + FunctionProtoType::ExtProtoInfo EPI = Spec.getEPI(); + DeclarationName Name = Context.DeclarationNames.getCXXConstructorName( Context.getCanonicalType(ClassType)); SourceLocation ClassLoc = ClassDecl->getLocation(); DeclarationNameInfo NameInfo(Name, ClassLoc); + + // An implicitly-declared copy constructor is an inline public + // member of its class. CXXConstructorDecl *CopyConstructor = CXXConstructorDecl::Create(Context, ClassDecl, ClassLoc, NameInfo, Context.getFunctionType(Context.VoidTy, @@ -6568,6 +6844,7 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor( /*isInline=*/true, /*isImplicitlyDeclared=*/true); CopyConstructor->setAccess(AS_public); + CopyConstructor->setDefaulted(); CopyConstructor->setTrivial(ClassDecl->hasTrivialCopyConstructor()); // Note that we have declared this constructor. @@ -6581,6 +6858,10 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor( SC_None, SC_None, 0); CopyConstructor->setParams(&FromParam, 1); + + if (ShouldDeleteCopyConstructor(CopyConstructor)) + CopyConstructor->setDeletedAsWritten(); + if (Scope *S = getScopeForContext(ClassDecl)) PushOnScopeChains(CopyConstructor, S, false); ClassDecl->addDecl(CopyConstructor); @@ -6589,10 +6870,9 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor( } void Sema::DefineImplicitCopyConstructor(SourceLocation CurrentLocation, - CXXConstructorDecl *CopyConstructor, - unsigned TypeQuals) { - assert((CopyConstructor->isImplicit() && - CopyConstructor->isCopyConstructor(TypeQuals) && + CXXConstructorDecl *CopyConstructor) { + assert((CopyConstructor->isDefaulted() && + CopyConstructor->isCopyConstructor() && !CopyConstructor->isUsed(false)) && "DefineImplicitCopyConstructor - call it for implicit copy ctor"); @@ -8040,14 +8320,24 @@ void Sema::SetDeclDefaulted(Decl *Dcl, SourceLocation DefaultLoc) { case CXXDefaultConstructor: { CXXConstructorDecl *CD = cast(MD); CheckExplicitlyDefaultedDefaultConstructor(CD); - DefineImplicitDefaultConstructor(DefaultLoc, CD); + if (!CD->isInvalidDecl()) + DefineImplicitDefaultConstructor(DefaultLoc, CD); + break; + } + + case CXXCopyConstructor: { + CXXConstructorDecl *CD = cast(MD); + CheckExplicitlyDefaultedCopyConstructor(CD); + if (!CD->isInvalidDecl()) + DefineImplicitCopyConstructor(DefaultLoc, CD); break; } case CXXDestructor: { CXXDestructorDecl *DD = cast(MD); CheckExplicitlyDefaultedDestructor(DD); - DefineImplicitDestructor(DefaultLoc, DD); + if (!DD->isInvalidDecl()) + DefineImplicitDestructor(DefaultLoc, DD); break; } diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 5a171eca98..8956d0f463 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -9852,16 +9852,15 @@ void Sema::MarkDeclarationReferenced(SourceLocation Loc, Decl *D) { // Note that this declaration has been used. if (CXXConstructorDecl *Constructor = dyn_cast(D)) { - unsigned TypeQuals; if (Constructor->isDefaulted() && Constructor->isDefaultConstructor()) { if (Constructor->isTrivial()) return; if (!Constructor->isUsed(false)) DefineImplicitDefaultConstructor(Loc, Constructor); } else if (Constructor->isImplicit() && - Constructor->isCopyConstructor(TypeQuals)) { + Constructor->isCopyConstructor()) { if (!Constructor->isUsed(false)) - DefineImplicitCopyConstructor(Loc, Constructor, TypeQuals); + DefineImplicitCopyConstructor(Loc, Constructor); } MarkVTableUsed(Loc, Constructor->getParent()); -- 2.40.0