replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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: 243594
This commit is contained in:
Richard Trieu 2015-07-29 23:47:19 +00:00
parent b98aa4359a
commit 1993dc8b15
3 changed files with 19 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
clang/lib/Sema/SemaInit.cpp
View File

@ -5944,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.
@ -5991,24 +5973,15 @@ static void CheckMoveOnConstruction(Sema &S, const Expr *InitExpr,
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();

8
clang/test/SemaCXX/warn-pessmizing-move.cpp
View File

@ -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>();
}
}

91
clang/test/SemaCXX/warn-redundant-move.cpp
View File

@ -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_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}}
@ -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@-1{{redundant move in return statement}}
// expected-note@-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@-1{{deleted here}}
};
struct F {
F(E);
// expected-note@-1{{passing argument to parameter here}}
};
F test6(E e) {
return e;
// expected-error@-1{{call to deleted constructor of 'E'}}
return std::move(e);
}
struct G {
G(G &&g);
// expected-note@-1{{copy constructor is implicitly deleted because 'G' has a user-declared move constructor}}
};
struct H {
H(G);
// expected-note@-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);
}