From: Sean Hunt Date: Tue, 21 Jun 2011 23:42:56 +0000 (+0000) Subject: Attempt to reapply this patch for caching copy assignment operator X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=661c67a9227708056850403847a10395308705e5;p=clang Attempt to reapply this patch for caching copy assignment operator lookup. Previously, it was breaking self-host, but it's been a week and a half and I can't reproduce, so I need to see if it's still failing. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@133581 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 0339a2941b..074207cb55 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1683,9 +1683,12 @@ public: DeclContextLookupResult LookupConstructors(CXXRecordDecl *Class); CXXConstructorDecl *LookupDefaultConstructor(CXXRecordDecl *Class); - CXXConstructorDecl *LookupCopyConstructor(CXXRecordDecl *Class, - unsigned Quals, - bool *ConstParam = 0); + CXXConstructorDecl *LookupCopyingConstructor(CXXRecordDecl *Class, + unsigned Quals, + bool *ConstParam = 0); + CXXMethodDecl *LookupCopyingAssignment(CXXRecordDecl *Class, unsigned Quals, + bool RValueThis, unsigned ThisQuals, + bool *ConstParam = 0); CXXDestructorDecl *LookupDestructor(CXXRecordDecl *Class); void ArgumentDependentLookup(DeclarationName Name, bool Operator, diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 571cbf2c24..a9b7ec317e 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -3637,7 +3637,7 @@ bool Sema::ShouldDeleteCopyConstructor(CXXConstructorDecl *CD) { // resolution, as applied to B's [copy] constructor, results in an // ambiguity or a function that is deleted or inaccessible from the // defaulted constructor - CXXConstructorDecl *BaseCtor = LookupCopyConstructor(BaseDecl, ArgQuals); + CXXConstructorDecl *BaseCtor = LookupCopyingConstructor(BaseDecl, ArgQuals); if (!BaseCtor || BaseCtor->isDeleted()) return true; if (CheckConstructorAccess(Loc, BaseCtor, BaseCtor->getAccess(), PDiag()) != @@ -3665,7 +3665,7 @@ bool Sema::ShouldDeleteCopyConstructor(CXXConstructorDecl *CD) { // resolution, as applied to B's [copy] constructor, results in an // ambiguity or a function that is deleted or inaccessible from the // defaulted constructor - CXXConstructorDecl *BaseCtor = LookupCopyConstructor(BaseDecl, ArgQuals); + CXXConstructorDecl *BaseCtor = LookupCopyingConstructor(BaseDecl, ArgQuals); if (!BaseCtor || BaseCtor->isDeleted()) return true; if (CheckConstructorAccess(Loc, BaseCtor, BaseCtor->getAccess(), PDiag()) != @@ -3727,8 +3727,8 @@ bool Sema::ShouldDeleteCopyConstructor(CXXConstructorDecl *CD) { // 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 - CXXConstructorDecl *FieldCtor = LookupCopyConstructor(FieldRecord, - ArgQuals); + CXXConstructorDecl *FieldCtor = LookupCopyingConstructor(FieldRecord, + ArgQuals); if (!FieldCtor || FieldCtor->isDeleted()) return true; if (CheckConstructorAccess(Loc, FieldCtor, FieldCtor->getAccess(), @@ -3753,19 +3753,15 @@ bool Sema::ShouldDeleteCopyAssignmentOperator(CXXMethodDecl *MD) { bool Union = RD->isUnion(); - bool ConstArg = - MD->getParamDecl(0)->getType()->getPointeeType().isConstQualified(); + unsigned ArgQuals = + MD->getParamDecl(0)->getType()->getPointeeType().isConstQualified() ? + Qualifiers::Const : 0; // We do this because we should never actually use an anonymous // union's constructor. if (Union && RD->isAnonymousStructOrUnion()) return false; - DeclarationName OperatorName = - Context.DeclarationNames.getCXXOperatorName(OO_Equal); - LookupResult R(*this, OperatorName, Loc, LookupOrdinaryName); - R.suppressDiagnostics(); - // FIXME: We should put some diagnostic logic right into this function. // C++0x [class.copy]/11 @@ -3787,37 +3783,11 @@ bool Sema::ShouldDeleteCopyAssignmentOperator(CXXMethodDecl *MD) { // resolution, as applied to B's [copy] assignment operator, results in // an ambiguity or a function that is deleted or inaccessible from the // assignment operator - - LookupQualifiedName(R, BaseDecl, false); - - // Filter out any result that isn't a copy-assignment operator. - LookupResult::Filter F = R.makeFilter(); - while (F.hasNext()) { - NamedDecl *D = F.next(); - if (CXXMethodDecl *Method = dyn_cast(D)) - if (Method->isCopyAssignmentOperator()) - continue; - - F.erase(); - } - F.done(); - - // Build a fake argument expression - QualType ArgType = BaseType; - QualType ThisType = BaseType; - if (ConstArg) - ArgType.addConst(); - Expr *Args[] = { new (Context) OpaqueValueExpr(Loc, ThisType, VK_LValue) - , new (Context) OpaqueValueExpr(Loc, ArgType, VK_LValue) - }; - - OverloadCandidateSet OCS((Loc)); - OverloadCandidateSet::iterator Best; - - AddFunctionCandidates(R.asUnresolvedSet(), Args, 2, OCS); - - if (OCS.BestViableFunction(*this, Loc, Best, false) != - OR_Success) + CXXMethodDecl *CopyOper = LookupCopyingAssignment(BaseDecl, ArgQuals, false, + 0); + if (!CopyOper || CopyOper->isDeleted()) + return true; + if (CheckDirectMemberAccess(Loc, CopyOper, PDiag()) != AR_accessible) return true; } @@ -3832,37 +3802,11 @@ bool Sema::ShouldDeleteCopyAssignmentOperator(CXXMethodDecl *MD) { // resolution, as applied to B's [copy] assignment operator, results in // an ambiguity or a function that is deleted or inaccessible from the // assignment operator - - LookupQualifiedName(R, BaseDecl, false); - - // Filter out any result that isn't a copy-assignment operator. - LookupResult::Filter F = R.makeFilter(); - while (F.hasNext()) { - NamedDecl *D = F.next(); - if (CXXMethodDecl *Method = dyn_cast(D)) - if (Method->isCopyAssignmentOperator()) - continue; - - F.erase(); - } - F.done(); - - // Build a fake argument expression - QualType ArgType = BaseType; - QualType ThisType = BaseType; - if (ConstArg) - ArgType.addConst(); - Expr *Args[] = { new (Context) OpaqueValueExpr(Loc, ThisType, VK_LValue) - , new (Context) OpaqueValueExpr(Loc, ArgType, VK_LValue) - }; - - OverloadCandidateSet OCS((Loc)); - OverloadCandidateSet::iterator Best; - - AddFunctionCandidates(R.asUnresolvedSet(), Args, 2, OCS); - - if (OCS.BestViableFunction(*this, Loc, Best, false) != - OR_Success) + CXXMethodDecl *CopyOper = LookupCopyingAssignment(BaseDecl, ArgQuals, false, + 0); + if (!CopyOper || CopyOper->isDeleted()) + return true; + if (CheckDirectMemberAccess(Loc, CopyOper, PDiag()) != AR_accessible) return true; } @@ -3909,37 +3853,12 @@ bool Sema::ShouldDeleteCopyAssignmentOperator(CXXMethodDecl *MD) { return true; } - LookupQualifiedName(R, FieldRecord, false); - - // Filter out any result that isn't a copy-assignment operator. - LookupResult::Filter F = R.makeFilter(); - while (F.hasNext()) { - NamedDecl *D = F.next(); - if (CXXMethodDecl *Method = dyn_cast(D)) - if (Method->isCopyAssignmentOperator()) - continue; - - F.erase(); - } - F.done(); - - // Build a fake argument expression - QualType ArgType = FieldType; - QualType ThisType = FieldType; - if (ConstArg) - ArgType.addConst(); - Expr *Args[] = { new (Context) OpaqueValueExpr(Loc, ThisType, VK_LValue) - , new (Context) OpaqueValueExpr(Loc, ArgType, VK_LValue) - }; - - OverloadCandidateSet OCS((Loc)); - OverloadCandidateSet::iterator Best; - - AddFunctionCandidates(R.asUnresolvedSet(), Args, 2, OCS); - - if (OCS.BestViableFunction(*this, Loc, Best, false) != - OR_Success) - return true; + CXXMethodDecl *CopyOper = LookupCopyingAssignment(FieldRecord, ArgQuals, + false, 0); + if (!CopyOper || CopyOper->isDeleted()) + return false; + if (CheckDirectMemberAccess(Loc, CopyOper, PDiag()) != AR_accessible) + return false; } } @@ -6688,58 +6607,6 @@ BuildSingleCopyAssign(Sema &S, SourceLocation Loc, QualType T, Loc, Copy.take()); } -/// \brief Determine whether the given class has a copy assignment operator -/// that accepts a const-qualified argument. -static bool hasConstCopyAssignment(Sema &S, const CXXRecordDecl *CClass) { - CXXRecordDecl *Class = const_cast(CClass); - - if (!Class->hasDeclaredCopyAssignment()) - S.DeclareImplicitCopyAssignment(Class); - - QualType ClassType = S.Context.getCanonicalType(S.Context.getTypeDeclType(Class)); - DeclarationName OpName - = S.Context.DeclarationNames.getCXXOperatorName(OO_Equal); - - DeclContext::lookup_const_iterator Op, OpEnd; - for (llvm::tie(Op, OpEnd) = Class->lookup(OpName); Op != OpEnd; ++Op) { - // C++ [class.copy]p9: - // A user-declared copy assignment operator is a non-static non-template - // member function of class X with exactly one parameter of type X, X&, - // const X&, volatile X& or const volatile X&. - const CXXMethodDecl* Method = dyn_cast(*Op); - if (!Method) - continue; - - if (Method->isStatic()) - continue; - if (Method->getPrimaryTemplate()) - continue; - const FunctionProtoType *FnType = - Method->getType()->getAs(); - assert(FnType && "Overloaded operator has no prototype."); - // Don't assert on this; an invalid decl might have been left in the AST. - if (FnType->getNumArgs() != 1 || FnType->isVariadic()) - continue; - bool AcceptsConst = true; - QualType ArgType = FnType->getArgType(0); - if (const LValueReferenceType *Ref = ArgType->getAs()){ - ArgType = Ref->getPointeeType(); - // Is it a non-const lvalue reference? - if (!ArgType.isConstQualified()) - AcceptsConst = false; - } - if (!S.Context.hasSameUnqualifiedType(ArgType, ClassType)) - continue; - - // We have a single argument of type cv X or cv X&, i.e. we've found the - // copy assignment operator. Return whether it accepts const arguments. - return AcceptsConst; - } - assert(Class->isInvalidDecl() && - "No copy assignment operator declared in valid code."); - return false; -} - std::pair Sema::ComputeDefaultedCopyAssignmentExceptionSpecAndConst( CXXRecordDecl *ClassDecl) { @@ -6760,11 +6627,28 @@ Sema::ComputeDefaultedCopyAssignmentExceptionSpecAndConst( for (CXXRecordDecl::base_class_iterator Base = ClassDecl->bases_begin(), BaseEnd = ClassDecl->bases_end(); HasConstCopyAssignment && Base != BaseEnd; ++Base) { + // We'll handle this below + if (LangOpts.CPlusPlus0x && Base->isVirtual()) + continue; + assert(!Base->getType()->isDependentType() && "Cannot generate implicit members for class with dependent bases."); - const CXXRecordDecl *BaseClassDecl - = cast(Base->getType()->getAs()->getDecl()); - HasConstCopyAssignment = hasConstCopyAssignment(*this, BaseClassDecl); + CXXRecordDecl *BaseClassDecl = Base->getType()->getAsCXXRecordDecl(); + LookupCopyingAssignment(BaseClassDecl, Qualifiers::Const, false, 0, + &HasConstCopyAssignment); + } + + // In C++0x, the above citation has "or virtual added" + if (LangOpts.CPlusPlus0x) { + for (CXXRecordDecl::base_class_iterator Base = ClassDecl->vbases_begin(), + BaseEnd = ClassDecl->vbases_end(); + HasConstCopyAssignment && Base != BaseEnd; ++Base) { + assert(!Base->getType()->isDependentType() && + "Cannot generate implicit members for class with dependent bases."); + CXXRecordDecl *BaseClassDecl = Base->getType()->getAsCXXRecordDecl(); + LookupCopyingAssignment(BaseClassDecl, Qualifiers::Const, false, 0, + &HasConstCopyAssignment); + } } // -- for all the nonstatic data members of X that are of a class @@ -6776,10 +6660,9 @@ Sema::ComputeDefaultedCopyAssignmentExceptionSpecAndConst( HasConstCopyAssignment && Field != FieldEnd; ++Field) { QualType FieldType = Context.getBaseElementType((*Field)->getType()); - if (const RecordType *FieldClassType = FieldType->getAs()) { - const CXXRecordDecl *FieldClassDecl - = cast(FieldClassType->getDecl()); - HasConstCopyAssignment = hasConstCopyAssignment(*this, FieldClassDecl); + if (CXXRecordDecl *FieldClassDecl = FieldType->getAsCXXRecordDecl()) { + LookupCopyingAssignment(FieldClassDecl, Qualifiers::Const, false, 0, + &HasConstCopyAssignment); } } @@ -6791,35 +6674,47 @@ Sema::ComputeDefaultedCopyAssignmentExceptionSpecAndConst( // C++ [except.spec]p14: // An implicitly declared special member function (Clause 12) shall have an // exception-specification. [...] + + // It is unspecified whether or not an implicit copy assignment operator + // attempts to deduplicate calls to assignment operators of virtual bases are + // made. As such, this exception specification is effectively unspecified. + // Based on a similar decision made for constness in C++0x, we're erring on + // the side of assuming such calls to be made regardless of whether they + // actually happen. ImplicitExceptionSpecification ExceptSpec(Context); + unsigned ArgQuals = HasConstCopyAssignment ? Qualifiers::Const : 0; for (CXXRecordDecl::base_class_iterator Base = ClassDecl->bases_begin(), BaseEnd = ClassDecl->bases_end(); Base != BaseEnd; ++Base) { + if (Base->isVirtual()) + continue; + CXXRecordDecl *BaseClassDecl = cast(Base->getType()->getAs()->getDecl()); - - if (!BaseClassDecl->hasDeclaredCopyAssignment()) - DeclareImplicitCopyAssignment(BaseClassDecl); + if (CXXMethodDecl *CopyAssign = LookupCopyingAssignment(BaseClassDecl, + ArgQuals, false, 0)) + ExceptSpec.CalledDecl(CopyAssign); + } - if (CXXMethodDecl *CopyAssign - = BaseClassDecl->getCopyAssignmentOperator(HasConstCopyAssignment)) + for (CXXRecordDecl::base_class_iterator Base = ClassDecl->vbases_begin(), + BaseEnd = ClassDecl->vbases_end(); + Base != BaseEnd; ++Base) { + CXXRecordDecl *BaseClassDecl + = cast(Base->getType()->getAs()->getDecl()); + if (CXXMethodDecl *CopyAssign = LookupCopyingAssignment(BaseClassDecl, + ArgQuals, false, 0)) ExceptSpec.CalledDecl(CopyAssign); } + for (CXXRecordDecl::field_iterator Field = ClassDecl->field_begin(), FieldEnd = ClassDecl->field_end(); Field != FieldEnd; ++Field) { QualType FieldType = Context.getBaseElementType((*Field)->getType()); - if (const RecordType *FieldClassType = FieldType->getAs()) { - CXXRecordDecl *FieldClassDecl - = cast(FieldClassType->getDecl()); - - if (!FieldClassDecl->hasDeclaredCopyAssignment()) - DeclareImplicitCopyAssignment(FieldClassDecl); - - if (CXXMethodDecl *CopyAssign - = FieldClassDecl->getCopyAssignmentOperator(HasConstCopyAssignment)) - ExceptSpec.CalledDecl(CopyAssign); + if (CXXRecordDecl *FieldClassDecl = FieldType->getAsCXXRecordDecl()) { + if (CXXMethodDecl *CopyAssign = + LookupCopyingAssignment(FieldClassDecl, ArgQuals, false, 0)) + ExceptSpec.CalledDecl(CopyAssign); } } @@ -7206,8 +7101,8 @@ Sema::ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl) { CXXRecordDecl *BaseClassDecl = cast(Base->getType()->getAs()->getDecl()); - LookupCopyConstructor(BaseClassDecl, Qualifiers::Const, - &HasConstCopyConstructor); + LookupCopyingConstructor(BaseClassDecl, Qualifiers::Const, + &HasConstCopyConstructor); } for (CXXRecordDecl::base_class_iterator Base = ClassDecl->vbases_begin(), @@ -7216,8 +7111,8 @@ Sema::ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl) { ++Base) { CXXRecordDecl *BaseClassDecl = cast(Base->getType()->getAs()->getDecl()); - LookupCopyConstructor(BaseClassDecl, Qualifiers::Const, - &HasConstCopyConstructor); + LookupCopyingConstructor(BaseClassDecl, Qualifiers::Const, + &HasConstCopyConstructor); } // -- for all the nonstatic data members of X that are of a @@ -7230,8 +7125,8 @@ Sema::ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl) { ++Field) { QualType FieldType = Context.getBaseElementType((*Field)->getType()); if (CXXRecordDecl *FieldClassDecl = FieldType->getAsCXXRecordDecl()) { - LookupCopyConstructor(FieldClassDecl, Qualifiers::Const, - &HasConstCopyConstructor); + LookupCopyingConstructor(FieldClassDecl, Qualifiers::Const, + &HasConstCopyConstructor); } } // Otherwise, the implicitly declared copy constructor will have @@ -7255,7 +7150,7 @@ Sema::ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl) { CXXRecordDecl *BaseClassDecl = cast(Base->getType()->getAs()->getDecl()); if (CXXConstructorDecl *CopyConstructor = - LookupCopyConstructor(BaseClassDecl, Quals)) + LookupCopyingConstructor(BaseClassDecl, Quals)) ExceptSpec.CalledDecl(CopyConstructor); } for (CXXRecordDecl::base_class_iterator Base = ClassDecl->vbases_begin(), @@ -7265,7 +7160,7 @@ Sema::ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl) { CXXRecordDecl *BaseClassDecl = cast(Base->getType()->getAs()->getDecl()); if (CXXConstructorDecl *CopyConstructor = - LookupCopyConstructor(BaseClassDecl, Quals)) + LookupCopyingConstructor(BaseClassDecl, Quals)) ExceptSpec.CalledDecl(CopyConstructor); } for (CXXRecordDecl::field_iterator Field = ClassDecl->field_begin(), @@ -7275,7 +7170,7 @@ Sema::ComputeDefaultedCopyCtorExceptionSpecAndConst(CXXRecordDecl *ClassDecl) { QualType FieldType = Context.getBaseElementType((*Field)->getType()); if (CXXRecordDecl *FieldClassDecl = FieldType->getAsCXXRecordDecl()) { if (CXXConstructorDecl *CopyConstructor = - LookupCopyConstructor(FieldClassDecl, Quals)) + LookupCopyingConstructor(FieldClassDecl, Quals)) ExceptSpec.CalledDecl(CopyConstructor); } } diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index 92ade1efcf..46058b69d1 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -2269,7 +2269,8 @@ Sema::SpecialMemberOverloadResult *Sema::LookupSpecialMember(CXXRecordDecl *D, (SM == CXXCopyConstructor && cast(M)->isCopyConstructor())) { QualType ArgType = M->getType()->getAs()->getArgType(0); - if (ArgType->getPointeeType().isConstQualified()) + if (!ArgType->isReferenceType() || + ArgType->getPointeeType().isConstQualified()) Result->setConstParamMatch(true); } } else { @@ -2310,10 +2311,10 @@ CXXConstructorDecl *Sema::LookupDefaultConstructor(CXXRecordDecl *Class) { return cast_or_null(Result->getMethod()); } -/// \brief Look up the copy constructor for the given class. -CXXConstructorDecl *Sema::LookupCopyConstructor(CXXRecordDecl *Class, - unsigned Quals, - bool *ConstParamMatch) { +/// \brief Look up the copying constructor for the given class. +CXXConstructorDecl *Sema::LookupCopyingConstructor(CXXRecordDecl *Class, + unsigned Quals, + bool *ConstParamMatch) { assert(!(Quals & ~(Qualifiers::Const | Qualifiers::Volatile)) && "non-const, non-volatile qualifiers for copy ctor arg"); SpecialMemberOverloadResult *Result = @@ -2341,6 +2342,27 @@ DeclContext::lookup_result Sema::LookupConstructors(CXXRecordDecl *Class) { return Class->lookup(Name); } +/// \brief Look up the copying assignment operator for the given class. +CXXMethodDecl *Sema::LookupCopyingAssignment(CXXRecordDecl *Class, + unsigned Quals, bool RValueThis, + unsigned ThisQuals, + bool *ConstParamMatch) { + assert(!(Quals & ~(Qualifiers::Const | Qualifiers::Volatile)) && + "non-const, non-volatile qualifiers for copy assignment arg"); + assert(!(ThisQuals & ~(Qualifiers::Const | Qualifiers::Volatile)) && + "non-const, non-volatile qualifiers for copy assignment this"); + SpecialMemberOverloadResult *Result = + LookupSpecialMember(Class, CXXCopyAssignment, Quals & Qualifiers::Const, + Quals & Qualifiers::Volatile, RValueThis, + ThisQuals & Qualifiers::Const, + ThisQuals & Qualifiers::Volatile); + + if (ConstParamMatch) + *ConstParamMatch = Result->hasConstParamMatch(); + + return Result->getMethod(); +} + /// \brief Look for the destructor of the given class. /// /// During semantic analysis, this routine should be used in lieu of