[cfe-commits] r167798 - in /cfe/trunk: lib/Sema/SemaDeclCXX.cpp test/CXX/special/class.copy/p28-cxx11.cpp test/CodeGenCXX/temporaries.cpp
Sean Silva
silvas at purdue.edu
Mon Nov 12 18:50:11 PST 2012
Does this impact any PR's?
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