[cfe-commits] r132841 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaLookup.cpp
Sean Hunt
scshunt at csclub.uwaterloo.ca
Fri Jun 10 02:24:41 PDT 2011
Author: coppro
Date: Fri Jun 10 04:24:41 2011
New Revision: 132841
URL: http://llvm.org/viewvc/llvm-project?rev=132841&view=rev
Log:
Implement caching of copy assignment operator lookup.
I believe, upon, careful review, that this code causes us to incorrectly
handle exception specifications of copy assignment operators in C++03
mode. However, we currently do not seem to properly implement the subtle
distinction between copying of members and bases made by implicit copy
constructors and assignment operators in C++03 - namely that they are
limited in their overload selection - in all cases. As such, I feel that
committing this code is correct pending a careful review of our
implementation of these semantics.
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=132841&r1=132840&r2=132841&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Jun 10 04:24:41 2011
@@ -1669,9 +1669,12 @@
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,
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=132841&r1=132840&r2=132841&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Jun 10 04:24:41 2011
@@ -3523,7 +3523,7 @@
// 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()) !=
@@ -3551,7 +3551,7 @@
// 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()) !=
@@ -3613,8 +3613,8 @@
// 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(),
@@ -3639,19 +3639,15 @@
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
@@ -3673,37 +3669,11 @@
// 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<CXXMethodDecl>(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;
}
@@ -3718,37 +3688,11 @@
// 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<CXXMethodDecl>(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;
}
@@ -3795,37 +3739,12 @@
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<CXXMethodDecl>(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;
}
}
@@ -6514,58 +6433,6 @@
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<CXXRecordDecl *>(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<CXXMethodDecl>(*Op);
- if (!Method)
- continue;
-
- if (Method->isStatic())
- continue;
- if (Method->getPrimaryTemplate())
- continue;
- const FunctionProtoType *FnType =
- Method->getType()->getAs<FunctionProtoType>();
- 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<LValueReferenceType>()){
- 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::ImplicitExceptionSpecification, bool>
Sema::ComputeDefaultedCopyAssignmentExceptionSpecAndConst(
CXXRecordDecl *ClassDecl) {
@@ -6586,11 +6453,28 @@
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<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->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
@@ -6602,10 +6486,9 @@
HasConstCopyAssignment && Field != FieldEnd;
++Field) {
QualType FieldType = Context.getBaseElementType((*Field)->getType());
- if (const RecordType *FieldClassType = FieldType->getAs<RecordType>()) {
- const CXXRecordDecl *FieldClassDecl
- = cast<CXXRecordDecl>(FieldClassType->getDecl());
- HasConstCopyAssignment = hasConstCopyAssignment(*this, FieldClassDecl);
+ if (CXXRecordDecl *FieldClassDecl = FieldType->getAsCXXRecordDecl()) {
+ LookupCopyingAssignment(FieldClassDecl, Qualifiers::Const, false, 0,
+ &HasConstCopyAssignment);
}
}
@@ -6617,35 +6500,47 @@
// 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<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->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<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->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<RecordType>()) {
- CXXRecordDecl *FieldClassDecl
- = cast<CXXRecordDecl>(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);
}
}
@@ -7029,8 +6924,8 @@
CXXRecordDecl *BaseClassDecl
= cast<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->getDecl());
- LookupCopyConstructor(BaseClassDecl, Qualifiers::Const,
- &HasConstCopyConstructor);
+ LookupCopyingConstructor(BaseClassDecl, Qualifiers::Const,
+ &HasConstCopyConstructor);
}
for (CXXRecordDecl::base_class_iterator Base = ClassDecl->vbases_begin(),
@@ -7039,8 +6934,8 @@
++Base) {
CXXRecordDecl *BaseClassDecl
= cast<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->getDecl());
- LookupCopyConstructor(BaseClassDecl, Qualifiers::Const,
- &HasConstCopyConstructor);
+ LookupCopyingConstructor(BaseClassDecl, Qualifiers::Const,
+ &HasConstCopyConstructor);
}
// -- for all the nonstatic data members of X that are of a
@@ -7053,8 +6948,8 @@
++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
@@ -7078,7 +6973,7 @@
CXXRecordDecl *BaseClassDecl
= cast<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->getDecl());
if (CXXConstructorDecl *CopyConstructor =
- LookupCopyConstructor(BaseClassDecl, Quals))
+ LookupCopyingConstructor(BaseClassDecl, Quals))
ExceptSpec.CalledDecl(CopyConstructor);
}
for (CXXRecordDecl::base_class_iterator Base = ClassDecl->vbases_begin(),
@@ -7088,7 +6983,7 @@
CXXRecordDecl *BaseClassDecl
= cast<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->getDecl());
if (CXXConstructorDecl *CopyConstructor =
- LookupCopyConstructor(BaseClassDecl, Quals))
+ LookupCopyingConstructor(BaseClassDecl, Quals))
ExceptSpec.CalledDecl(CopyConstructor);
}
for (CXXRecordDecl::field_iterator Field = ClassDecl->field_begin(),
@@ -7098,7 +6993,7 @@
QualType FieldType = Context.getBaseElementType((*Field)->getType());
if (CXXRecordDecl *FieldClassDecl = FieldType->getAsCXXRecordDecl()) {
if (CXXConstructorDecl *CopyConstructor =
- LookupCopyConstructor(FieldClassDecl, Quals))
+ LookupCopyingConstructor(FieldClassDecl, Quals))
ExceptSpec.CalledDecl(CopyConstructor);
}
}
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=132841&r1=132840&r2=132841&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri Jun 10 04:24:41 2011
@@ -2269,7 +2269,8 @@
(SM == CXXCopyConstructor &&
cast<CXXConstructorDecl>(M)->isCopyConstructor())) {
QualType ArgType = M->getType()->getAs<FunctionProtoType>()->getArgType(0);
- if (ArgType->getPointeeType().isConstQualified())
+ if (!ArgType->isReferenceType() ||
+ ArgType->getPointeeType().isConstQualified())
Result->setConstParamMatch(true);
}
} else {
@@ -2310,10 +2311,10 @@
return cast_or_null<CXXConstructorDecl>(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 @@
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
More information about the cfe-commits
mailing list