[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