[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: [PATCH] D11615: Fix redundant move warning
From: Richard Trieu <rtrieu () google ! com>
Date: 2015-07-29 23:41:38
Message-ID: differential-rev-PHID-DREV-xvylcegrzteh6nemih6n-req () reviews ! llvm ! org
[Download RAW message or body]
rtrieu created this revision.
rtrieu added a reviewer: rsmith.
rtrieu added subscribers: CornedBee, arthur.j.odwyer, cfe-commits.
Until DR1579 is implemented, redundant move should only be emitted when a p=
arameter variable has the same type as the function return type and is retu=
rned. This removes the case where the returned type and the returned varia=
ble have different types. Also include a test to check for that the move c=
onstructor is used by checking the AST dump.
http://reviews.llvm.org/D11615
Files:
lib/Sema/SemaInit.cpp
test/SemaCXX/warn-pessmizing-move.cpp
test/SemaCXX/warn-redundant-move.cpp
["D11615.30967.patch" (text/x-patch)]
Index: test/SemaCXX/warn-redundant-move.cpp
===================================================================
--- test/SemaCXX/warn-redundant-move.cpp
+++ 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,23 +13,16 @@
}
}
+// test1 and test2 should not warn until after implementation of DR1579.
struct A {};
struct B : public A {};
A test1(B b1) {
B b2;
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 @@
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 @@
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,72 +89,28 @@
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}}
// 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>();
- 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);
-}
Index: test/SemaCXX/warn-pessmizing-move.cpp
===================================================================
--- test/SemaCXX/warn-pessmizing-move.cpp
+++ test/SemaCXX/warn-pessmizing-move.cpp
@@ -200,8 +200,6 @@
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,20 +214,16 @@
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;
return std::move(t);
}
void run_test2() {
test2<A, A>();
test2<B, A>();
- test2<D, C>();
}
}
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -5944,24 +5944,6 @@
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 @@
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();
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic