[cfe-commits] r167798 - in /cfe/trunk: lib/Sema/SemaDeclCXX.cpp test/CXX/special/class.copy/p28-cxx11.cpp test/CodeGenCXX/temporaries.cpp
Richard Smith
richard at metafoo.co.uk
Mon Nov 12 19:40:33 PST 2012
On Mon, Nov 12, 2012 at 6:50 PM, Sean Silva <silvas at purdue.edu> wrote:
> Does this impact any PR's?
None that I'm aware of, but I've not looked very hard.
> On Mon, Nov 12, 2012 at 7:54 PM, Richard Smith
> <richard-llvm at metafoo.co.uk> wrote:
>> Author: rsmith
>> Date: Mon Nov 12 18:54:12 2012
>> New Revision: 167798
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=167798&view=rev
>> Log:
>> Fix some wrong-code bugs in implicitly-defined assignment operators:
>> - In C++11, perform overload resolution over all assignment operators, rather than just looking for copy/move assignment operators.
>> - Clean up after temporaries produced by operator= immediately, rather than accumulating them until the end of the function.
>>
>> Added:
>> cfe/trunk/test/CXX/special/class.copy/p28-cxx11.cpp
>> Modified:
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/test/CodeGenCXX/temporaries.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=167798&r1=167797&r2=167798&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Nov 12 18:54:12 2012
>> @@ -7505,7 +7505,7 @@
>> return S.Owned(Call.takeAs<Stmt>());
>> }
>>
>> - // C++0x [class.copy]p28:
>> + // C++11 [class.copy]p28:
>> // Each subobject is assigned in the manner appropriate to its type:
>> //
>> // - if the subobject is of class type, as if by a call to operator= with
>> @@ -7513,34 +7513,41 @@
>> // subobject of x as a single function argument (as if by explicit
>> // qualification; that is, ignoring any possible virtual overriding
>> // functions in more derived classes);
>> + //
>> + // C++03 [class.copy]p13:
>> + // - if the subobject is of class type, the copy assignment operator for
>> + // the class is used (as if by explicit qualification; that is,
>> + // ignoring any possible virtual overriding functions in more derived
>> + // classes);
>> if (const RecordType *RecordTy = T->getAs<RecordType>()) {
>> CXXRecordDecl *ClassDecl = cast<CXXRecordDecl>(RecordTy->getDecl());
>> -
>> +
>> // Look for operator=.
>> DeclarationName Name
>> = S.Context.DeclarationNames.getCXXOperatorName(OO_Equal);
>> LookupResult OpLookup(S, Name, Loc, Sema::LookupOrdinaryName);
>> S.LookupQualifiedName(OpLookup, ClassDecl, false);
>> -
>> - // Filter out any result that isn't a copy/move-assignment operator.
>> - // FIXME: This is wrong in C++11. We should perform overload resolution
>> - // instead here.
>> - LookupResult::Filter F = OpLookup.makeFilter();
>> - while (F.hasNext()) {
>> - NamedDecl *D = F.next();
>> - if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D))
>> - if (Method->isCopyAssignmentOperator() ||
>> - (!Copying && Method->isMoveAssignmentOperator()))
>> - continue;
>>
>> - F.erase();
>> + // Prior to C++11, filter out any result that isn't a copy/move-assignment
>> + // operator.
>> + if (!S.getLangOpts().CPlusPlus0x) {
>> + LookupResult::Filter F = OpLookup.makeFilter();
>> + while (F.hasNext()) {
>> + NamedDecl *D = F.next();
>> + if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D))
>> + if (Method->isCopyAssignmentOperator() ||
>> + (!Copying && Method->isMoveAssignmentOperator()))
>> + continue;
>> +
>> + F.erase();
>> + }
>> + F.done();
>> }
>> - F.done();
>> -
>> +
>> // Suppress the protected check (C++ [class.protected]) for each of the
>> - // assignment operators we found. This strange dance is required when
>> + // assignment operators we found. This strange dance is required when
>> // we're assigning via a base classes's copy-assignment operator. To
>> - // ensure that we're getting the right base class subobject (without
>> + // ensure that we're getting the right base class subobject (without
>> // ambiguities), we need to cast "this" to that subobject type; to
>> // ensure that we don't go through the virtual call mechanism, we need
>> // to qualify the operator= name with the base class (see below). However,
>> @@ -7555,20 +7562,20 @@
>> L.setAccess(AS_public);
>> }
>> }
>> -
>> +
>> // Create the nested-name-specifier that will be used to qualify the
>> // reference to operator=; this is required to suppress the virtual
>> // call mechanism.
>> CXXScopeSpec SS;
>> const Type *CanonicalT = S.Context.getCanonicalType(T.getTypePtr());
>> - SS.MakeTrivial(S.Context,
>> - NestedNameSpecifier::Create(S.Context, 0, false,
>> + SS.MakeTrivial(S.Context,
>> + NestedNameSpecifier::Create(S.Context, 0, false,
>> CanonicalT),
>> Loc);
>> -
>> +
>> // Create the reference to operator=.
>> ExprResult OpEqualRef
>> - = S.BuildMemberReferenceExpr(To, T, Loc, /*isArrow=*/false, SS,
>> + = S.BuildMemberReferenceExpr(To, T, Loc, /*isArrow=*/false, SS,
>> /*TemplateKWLoc=*/SourceLocation(),
>> /*FirstQualifierInScope=*/0,
>> OpLookup,
>> @@ -7576,33 +7583,34 @@
>> /*SuppressQualifierCheck=*/true);
>> if (OpEqualRef.isInvalid())
>> return StmtError();
>> -
>> +
>> // Build the call to the assignment operator.
>>
>> - ExprResult Call = S.BuildCallToMemberFunction(/*Scope=*/0,
>> + ExprResult Call = S.BuildCallToMemberFunction(/*Scope=*/0,
>> OpEqualRef.takeAs<Expr>(),
>> Loc, &From, 1, Loc);
>> if (Call.isInvalid())
>> return StmtError();
>> -
>> - // FIXME: ActOnFinishFullExpr, ActOnExprStmt.
>> - return S.Owned(Call.takeAs<Stmt>());
>> +
>> + // Convert to an expression-statement, and clean up any produced
>> + // temporaries.
>> + return S.ActOnExprStmt(S.MakeFullExpr(Call.take(), Loc));
>> }
>>
>> - // - if the subobject is of scalar type, the built-in assignment
>> + // - if the subobject is of scalar type, the built-in assignment
>> // operator is used.
>> - const ConstantArrayType *ArrayTy = S.Context.getAsConstantArrayType(T);
>> + const ConstantArrayType *ArrayTy = S.Context.getAsConstantArrayType(T);
>> if (!ArrayTy) {
>> ExprResult Assignment = S.CreateBuiltinBinOp(Loc, BO_Assign, To, From);
>> if (Assignment.isInvalid())
>> return StmtError();
>> -
>> - return S.Owned(Assignment.takeAs<Stmt>());
>> +
>> + return S.ActOnExprStmt(S.MakeFullExpr(Assignment.take(), Loc));
>> }
>> -
>> - // - if the subobject is an array, each element is assigned, in the
>> +
>> + // - if the subobject is an array, each element is assigned, in the
>> // manner appropriate to the element type;
>> -
>> +
>> // Construct a loop over the array bounds, e.g.,
>> //
>> // for (__SIZE_TYPE__ i0 = 0; i0 != array-size; ++i0)
>>
>> Added: cfe/trunk/test/CXX/special/class.copy/p28-cxx11.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/p28-cxx11.cpp?rev=167798&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/CXX/special/class.copy/p28-cxx11.cpp (added)
>> +++ cfe/trunk/test/CXX/special/class.copy/p28-cxx11.cpp Mon Nov 12 18:54:12 2012
>> @@ -0,0 +1,19 @@
>> +// RUN: %clang_cc1 -std=c++98 %s -fsyntax-only
>> +// RUN: %clang_cc1 -std=c++11 %s -verify
>> +
>> +// In C++11, we must perform overload resolution to determine which function is
>> +// called by a defaulted assignment operator, and the selected operator might
>> +// not be a copy or move assignment (it might be a specialization of a templated
>> +// 'operator=', for instance).
>> +struct A {
>> + A &operator=(const A &);
>> +
>> + template<typename T>
>> + A &operator=(T &&) { return T::error; } // expected-error {{no member named 'error' in 'A'}}
>> +};
>> +
>> +struct B : A {
>> + B &operator=(B &&);
>> +};
>> +
>> +B &B::operator=(B &&) = default; // expected-note {{here}}
>>
>> Modified: cfe/trunk/test/CodeGenCXX/temporaries.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/temporaries.cpp?rev=167798&r1=167797&r2=167798&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/temporaries.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/temporaries.cpp Mon Nov 12 18:54:12 2012
>> @@ -537,3 +537,24 @@
>> (void) (A [3]) {};
>> }
>> }
>> +
>> +namespace AssignmentOp {
>> + struct A { ~A(); };
>> + struct B { A operator=(const B&); };
>> + struct C : B { B b1, b2; };
>> + // CHECK: define void @_ZN12AssignmentOp1fE
>> + void f(C &c1, const C &c2) {
>> + // CHECK: call {{.*}} @_ZN12AssignmentOp1CaSERKS0_(
>> + c1 = c2;
>> + }
>> +
>> + // Ensure that each 'A' temporary is destroyed before the next subobject is
>> + // copied.
>> + // CHECK: define {{.*}} @_ZN12AssignmentOp1CaSERKS0_(
>> + // CHECK: call {{.*}} @_ZN12AssignmentOp1BaSERKS
>> + // CHECK: call {{.*}} @_ZN12AssignmentOp1AD1Ev(
>> + // CHECK: call {{.*}} @_ZN12AssignmentOp1BaSERKS
>> + // CHECK: call {{.*}} @_ZN12AssignmentOp1AD1Ev(
>> + // CHECK: call {{.*}} @_ZN12AssignmentOp1BaSERKS
>> + // CHECK: call {{.*}} @_ZN12AssignmentOp1AD1Ev(
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list