From 0e4cb2fe80014791ff741075699cf224363ac00c Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Wed, 29 Jul 2015 23:47:19 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@243594 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaInit.cpp | 33 +--------- test/SemaCXX/warn-pessmizing-move.cpp | 8 +-- test/SemaCXX/warn-redundant-move.cpp | 91 +++++---------------------- 3 files changed, 19 insertions(+), 113 deletions(-) diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index f66bc7f37a..adec512693 100644 --- a/lib/Sema/SemaInit.cpp +++ b/lib/Sema/SemaInit.cpp @@ -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(InitExpr)) { - InitExpr = MTE->GetTemporaryExpr()->IgnoreImpCasts(); - while (const CXXConstructExpr *CCE = - dyn_cast(InitExpr)) { - if (isa(CCE)) - return; - if (CCE->getNumArgs() == 0) - return; - if (CCE->getNumArgs() > 1 && !isa(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(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(); diff --git a/test/SemaCXX/warn-pessmizing-move.cpp b/test/SemaCXX/warn-pessmizing-move.cpp index 5645564c74..71e99dca00 100644 --- a/test/SemaCXX/warn-pessmizing-move.cpp +++ b/test/SemaCXX/warn-pessmizing-move.cpp @@ -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 @@ -216,12 +214,9 @@ namespace templates { void run_test1() { test1(); test1(); - test1(); - test1(); } - // 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 T1 test2() { T2 t; @@ -230,6 +225,5 @@ namespace templates { void run_test2() { test2(); test2(); - test2(); } } diff --git a/test/SemaCXX/warn-redundant-move.cpp b/test/SemaCXX/warn-redundant-move.cpp index d160695207..abfb001fa8 100644 --- a/test/SemaCXX/warn-redundant-move.cpp +++ b/test/SemaCXX/warn-redundant-move.cpp @@ -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 typename remove_reference::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 - 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(); - test1(); - test1(); - test1(); + test1(A()); + test1(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 - T1 test2() { - T2 t; + T1 test2(T2 t) { return std::move(t); } void run_test2() { - test2(); - test2(); - test2(); + test2(A()); + test2(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); -} -- 2.40.0