Commit 78356d85 authored by Hans Wennborg's avatar Hans Wennborg
Browse files

Merging r243463, r243538, and r243594: 

------------------------------------------------------------------------
r243463 | rtrieu | 2015-07-28 12:06:16 -0700 (Tue, 28 Jul 2015) | 6 lines

Do not give a -Wredundant-move warning when removing the move will result in an
error.

If the object being moved has a move constructor and a deleted copy constructor,
std::move is required, otherwise Clang will give a deleted constructor error.
------------------------------------------------------------------------

------------------------------------------------------------------------
r243538 | rtrieu | 2015-07-29 10:03:34 -0700 (Wed, 29 Jul 2015) | 7 lines

Disable -Wpessimizing-move and -Wredundant-move in template instantiations.

Dependent types can throw off the analysis for these warnings, possibly giving
conflicting warnings and fix-its.  Disabling the warning in template
instantiations will prevent this problem, and will still catch the
non-dependent cases in templates.
------------------------------------------------------------------------

------------------------------------------------------------------------
r243594 | rtrieu | 2015-07-29 16:47:19 -0700 (Wed, 29 Jul 2015) | 7 lines

Fix -Wredundant-move warning.

Without DR1579 implemented, the only case for -Wredundant-move is for a
parameter being returned with the same type as the function return type.  Also
include a check to verify that the move constructor will be used by matching
nodes in the AST dump.
------------------------------------------------------------------------

llvm-svn: 243607
parent 4ca52e9a

clang/lib/Sema/SemaInit.cpp

+11 −25
Original line number Diff line number Diff line
@@ -5926,6 +5926,9 @@ static void CheckMoveOnConstruction(Sema &S, const Expr *InitExpr, 
  if (!InitExpr)

    return;



  if (!S.ActiveTemplateInstantiations.empty())

    return;



  QualType DestType = InitExpr->getType();

  if (!DestType->isRecordType())

    return;
@@ -5941,24 +5944,6 @@ static void CheckMoveOnConstruction(Sema &S, const Expr *InitExpr, 
      return;



    InitExpr = CCE->getArg(0)->IgnoreImpCasts();



    // Remove implicit temporary and constructor nodes.

    if (const MaterializeTemporaryExpr *MTE =

            dyn_cast<MaterializeTemporaryExpr>(InitExpr)) {

      InitExpr = MTE->GetTemporaryExpr()->IgnoreImpCasts();

      while (const CXXConstructExpr *CCE =

                 dyn_cast<CXXConstructExpr>(InitExpr)) {

        if (isa<CXXTemporaryObjectExpr>(CCE))

          return;

        if (CCE->getNumArgs() == 0)

          return;

        if (CCE->getNumArgs() > 1 && !isa<CXXDefaultArgExpr>(CCE->getArg(1)))

          return;

        InitExpr = CCE->getArg(0);

      }

      InitExpr = InitExpr->IgnoreImpCasts();

      DiagID = diag::warn_redundant_move_on_return;

    }

  }



  // Find the std::move call and get the argument.
@@ -5983,19 +5968,20 @@ static void CheckMoveOnConstruction(Sema &S, const Expr *InitExpr, 
    if (!VD || !VD->hasLocalStorage())

      return;



    if (!VD->getType()->isRecordType())

    QualType SourceType = VD->getType();

    if (!SourceType->isRecordType())

      return;



    if (!S.Context.hasSameUnqualifiedType(DestType, SourceType)) {

      return;

    }



    // If we're returning a function parameter, copy elision

    // is not possible.

    if (isa<ParmVarDecl>(VD))

      DiagID = diag::warn_redundant_move_on_return;



    if (DiagID == 0) {

      DiagID = S.Context.hasSameUnqualifiedType(DestType, VD->getType())

                   ? diag::warn_pessimizing_move_on_return

                   : diag::warn_redundant_move_on_return;

    }

    else

      DiagID = diag::warn_pessimizing_move_on_return;

  } else {

    DiagID = diag::warn_pessimizing_move_on_initialization;

    const Expr *ArgStripped = Arg->IgnoreImplicit()->IgnoreParens();

clang/test/SemaCXX/warn-pessmizing-move.cpp

+31 −0
Original line number Diff line number Diff line
@@ -196,3 +196,34 @@ A test10() { 
  // expected-warning@-1{{prevents copy elision}}

  // CHECK-NOT: fix-it

}



namespace templates {

  struct A {};

  struct B { B(A); };



  // Warn once here since the type is not dependent.

  template <typename T>

  A test1() {

    A a;

    return std::move(a);

    // expected-warning@-1{{prevents copy elision}}

    // expected-note@-2{{remove std::move call}}

    // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:22}:""

    // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:""

  }

  void run_test1() {

    test1<A>();

    test1<B>();

  }



  // T1 and T2 may not be the same, the warning may not always apply.

  template <typename T1, typename T2>

  T1 test2() {

    T2 t;

    return std::move(t);

  }

  void run_test2() {

    test2<A, A>();

    test2<B, A>();

  }

}

clang/test/SemaCXX/warn-redundant-move.cpp

+36 −24
Original line number Diff line number Diff line 
// RUN: %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 -verify %s

// RUN: %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s

// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -ast-dump | FileCheck %s --check-prefix=CHECK-AST



// definitions for std::move

namespace std {
@@ -12,6 +13,7 @@ template <class T> typename remove_reference<T>::type &&move(T &&t); 
}

}



// test1 and test2 should not warn until after implementation of DR1579.

struct A {};

struct B : public A {};
@@ -20,15 +22,7 @@ A test1(B b1) { 
  return b1;

  return b2;

  return std::move(b1);

  // expected-warning@-1{{redundant move}}

  // expected-note@-2{{remove std::move call}}

  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""

  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:""

  return std::move(b2);

  // expected-warning@-1{{redundant move}}

  // expected-note@-2{{remove std::move call}}

  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""

  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:""

}



struct C {
@@ -46,25 +40,9 @@ C test2(A a1, B b1) { 
  return b2;



  return std::move(a1);

  // expected-warning@-1{{redundant move}}

  // expected-note@-2{{remove std::move call}}

  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""

  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:""

  return std::move(a2);

  // expected-warning@-1{{redundant move}}

  // expected-note@-2{{remove std::move call}}

  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""

  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:""

  return std::move(b1);

  // expected-warning@-1{{redundant move}}

  // expected-note@-2{{remove std::move call}}

  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""

  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:""

  return std::move(b2);

  // expected-warning@-1{{redundant move}}

  // expected-note@-2{{remove std::move call}}

  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""

  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:""

}



// Copy of tests above with types changed to reference types.
@@ -95,6 +73,11 @@ C test4(A& a1, B& b1) { 
struct D {};

D test5(D d) {

  return d;

  // Verify the implicit move from the AST dump

  // CHECK-AST: ReturnStmt{{.*}}line:[[@LINE-2]]

  // CHECK-AST-NEXT: CXXConstructExpr{{.*}}struct D{{.*}}void (struct D &&)

  // CHECK-AST-NEXT: ImplicitCastExpr

  // CHECK-AST-NEXT: DeclRefExpr{{.*}}ParmVar{{.*}}'d'



  return std::move(d);

  // expected-warning@-1{{redundant move in return statement}}
@@ -102,3 +85,32 @@ D test5(D d) { 
  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""

  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:22}:""

}



namespace templates {

  struct A {};

  struct B { B(A); };



  // Warn once here since the type is not dependent.

  template <typename T>

  A test1(A a) {

    return std::move(a);

    // expected-warning@-1{{redundant move in return statement}}

    // expected-note@-2{{remove std::move call here}}

    // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:22}:""

    // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:""

  }

  void run_test1() {

    test1<A>(A());

    test1<B>(A());

  }



  // T1 and T2 may not be the same, the warning may not always apply.

  template <typename T1, typename T2>

  T1 test2(T2 t) {

    return std::move(t);

  }

  void run_test2() {

    test2<A, A>(A());

    test2<B, A>(A());

  }

}