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
parameter variable has the same type as the function return type and is
returned. This removes the case where the returned type and the returned
variable have different types. Also include a test to check for that the move
constructor 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
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
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
