r243594 - Fix -Wredundant-move warning.
Richard Trieu
rtrieu at google.com
Wed Jul 29 16:47:19 PDT 2015
Author: rtrieu
Date: Wed Jul 29 18:47:19 2015
New Revision: 243594
URL: http://llvm.org/viewvc/llvm-project?rev=243594&view=rev
Log:
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.
Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp
cfe/trunk/test/SemaCXX/warn-redundant-move.cpp
Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=243594&r1=243593&r2=243594&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Wed Jul 29 18:47:19 2015
@@ -5944,24 +5944,6 @@ static void CheckMoveOnConstruction(Sema
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.
@@ -5991,24 +5973,15 @@ static void CheckMoveOnConstruction(Sema
return;
if (!S.Context.hasSameUnqualifiedType(DestType, SourceType)) {
- if (CXXRecordDecl *RD = SourceType->getAsCXXRecordDecl()) {
- for (auto* Construct : RD->ctors()) {
- if (Construct->isCopyConstructor() && Construct->isDeleted())
- return;
- }
- }
+ 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();
Modified: cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp?rev=243594&r1=243593&r2=243594&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp Wed Jul 29 18:47:19 2015
@@ -200,8 +200,6 @@ A test10() {
namespace templates {
struct A {};
struct B { B(A); };
- struct C { C(); C(C&&); };
- struct D { D(C); };
// Warn once here since the type is not dependent.
template <typename T>
@@ -216,12 +214,9 @@ namespace templates {
void run_test1() {
test1<A>();
test1<B>();
- test1<C>();
- test1<D>();
}
- // Either a pessimizing move, a redundant move, or no warning could be
- // emitted, given the right types. So just drop the warning.
+ // T1 and T2 may not be the same, the warning may not always apply.
template <typename T1, typename T2>
T1 test2() {
T2 t;
@@ -230,6 +225,5 @@ namespace templates {
void run_test2() {
test2<A, A>();
test2<B, A>();
- test2<D, C>();
}
}
Modified: cfe/trunk/test/SemaCXX/warn-redundant-move.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-redundant-move.cpp?rev=243594&r1=243593&r2=243594&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-redundant-move.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-redundant-move.cpp Wed Jul 29 18:47:19 2015
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %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_refer
}
}
+// 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 at -1{{redundant move}}
- // expected-note at -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 at -1{{redundant move}}
- // expected-note at -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 at -1{{redundant move}}
- // expected-note at -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 at -1{{redundant move}}
- // expected-note at -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 at -1{{redundant move}}
- // expected-note at -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 at -1{{redundant move}}
- // expected-note at -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 at -1{{redundant move in return statement}}
@@ -106,13 +89,10 @@ D test5(D d) {
namespace templates {
struct A {};
struct B { B(A); };
- struct C { C(); C(C&&); };
- struct D { D(C); };
// Warn once here since the type is not dependent.
template <typename T>
- B test1() {
- A a;
+ A test1(A a) {
return std::move(a);
// expected-warning at -1{{redundant move in return statement}}
// expected-note at -2{{remove std::move call here}}
@@ -120,58 +100,17 @@ namespace templates {
// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:""
}
void run_test1() {
- test1<A>();
- test1<B>();
- test1<C>();
- test1<D>();
+ test1<A>(A());
+ test1<B>(A());
}
- // Either a pessimizing move, a redundant move, or no warning could be
- // emitted, given the right types. So just drop the warning.
+ // T1 and T2 may not be the same, the warning may not always apply.
template <typename T1, typename T2>
- T1 test2() {
- T2 t;
+ T1 test2(T2 t) {
return std::move(t);
}
void run_test2() {
- test2<A, A>();
- test2<B, A>();
- test2<D, C>();
+ test2<A, A>(A());
+ test2<B, A>(A());
}
}
-
-// No more fix-its past here.
-// CHECK-NOT: fix-it
-
-// A deleted copy constructor will prevent moves without std::move
-struct E {
- E(E &&e);
- E(const E &e) = delete;
- // expected-note at -1{{deleted here}}
-};
-
-struct F {
- F(E);
- // expected-note at -1{{passing argument to parameter here}}
-};
-
-F test6(E e) {
- return e;
- // expected-error at -1{{call to deleted constructor of 'E'}}
- return std::move(e);
-}
-
-struct G {
- G(G &&g);
- // expected-note at -1{{copy constructor is implicitly deleted because 'G' has a user-declared move constructor}}
-};
-
-struct H {
- H(G);
- // expected-note at -1{{passing argument to parameter here}}
-};
-
-H test6(G g) {
- return g; // expected-error{{call to implicitly-deleted copy constructor of 'G'}}
- return std::move(g);
-}
More information about the cfe-commits
mailing list