r373421 - Revert r368237 - Update fix-it hints for std::move warnings.
Richard Trieu via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 1 19:32:15 PDT 2019
Author: rtrieu
Date: Tue Oct 1 19:32:15 2019
New Revision: 373421
URL: http://llvm.org/viewvc/llvm-project?rev=373421&view=rev
Log:
Revert r368237 - Update fix-it hints for std::move warnings.
r368237 attempted to improve fix-its for move warnings, but introduced some
regressions to -Wpessimizing-move. Revert that change and add the missing
test cases to the pessimizing move test to prevent future regressions.
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=373421&r1=373420&r2=373421&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Tue Oct 1 19:32:15 2019
@@ -7580,34 +7580,27 @@ static void CheckMoveOnConstruction(Sema
if (!DestType->isRecordType())
return;
- const CXXConstructExpr *CCE =
- dyn_cast<CXXConstructExpr>(InitExpr->IgnoreParens());
- if (!CCE || CCE->getNumArgs() != 1)
- return;
+ unsigned DiagID = 0;
+ if (IsReturnStmt) {
+ const CXXConstructExpr *CCE =
+ dyn_cast<CXXConstructExpr>(InitExpr->IgnoreParens());
+ if (!CCE || CCE->getNumArgs() != 1)
+ return;
- if (!CCE->getConstructor()->isCopyOrMoveConstructor())
- return;
+ if (!CCE->getConstructor()->isCopyOrMoveConstructor())
+ return;
- InitExpr = CCE->getArg(0)->IgnoreImpCasts();
+ InitExpr = CCE->getArg(0)->IgnoreImpCasts();
+ }
// Find the std::move call and get the argument.
const CallExpr *CE = dyn_cast<CallExpr>(InitExpr->IgnoreParens());
if (!CE || !CE->isCallToStdMove())
return;
- const Expr *Arg = CE->getArg(0);
-
- unsigned DiagID = 0;
-
- if (!IsReturnStmt && !isa<MaterializeTemporaryExpr>(Arg))
- return;
+ const Expr *Arg = CE->getArg(0)->IgnoreImplicit();
- if (isa<MaterializeTemporaryExpr>(Arg)) {
- DiagID = diag::warn_pessimizing_move_on_initialization;
- const Expr *ArgStripped = Arg->IgnoreImplicit()->IgnoreParens();
- if (!ArgStripped->isRValue() || !ArgStripped->getType()->isRecordType())
- return;
- } else { // IsReturnStmt
+ if (IsReturnStmt) {
const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Arg->IgnoreParenImpCasts());
if (!DRE || DRE->refersToEnclosingVariableOrCapture())
return;
@@ -7634,18 +7627,24 @@ static void CheckMoveOnConstruction(Sema
DiagID = 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();
+ if (!ArgStripped->isRValue() || !ArgStripped->getType()->isRecordType())
+ return;
}
S.Diag(CE->getBeginLoc(), DiagID);
// Get all the locations for a fix-it. Don't emit the fix-it if any location
// is within a macro.
- SourceLocation BeginLoc = CCE->getBeginLoc();
- if (BeginLoc.isMacroID())
+ SourceLocation CallBegin = CE->getCallee()->getBeginLoc();
+ if (CallBegin.isMacroID())
return;
SourceLocation RParen = CE->getRParenLoc();
if (RParen.isMacroID())
return;
+ SourceLocation LParen;
SourceLocation ArgLoc = Arg->getBeginLoc();
// Special testing for the argument location. Since the fix-it needs the
@@ -7656,16 +7655,14 @@ static void CheckMoveOnConstruction(Sema
ArgLoc = S.getSourceManager().getImmediateExpansionRange(ArgLoc).getBegin();
}
- SourceLocation LParen = ArgLoc.getLocWithOffset(-1);
if (LParen.isMacroID())
return;
- SourceLocation EndLoc = CCE->getEndLoc();
- if (EndLoc.isMacroID())
- return;
+
+ LParen = ArgLoc.getLocWithOffset(-1);
S.Diag(CE->getBeginLoc(), diag::note_remove_move)
- << FixItHint::CreateRemoval(SourceRange(BeginLoc, LParen))
- << FixItHint::CreateRemoval(SourceRange(RParen, EndLoc));
+ << FixItHint::CreateRemoval(SourceRange(CallBegin, LParen))
+ << FixItHint::CreateRemoval(SourceRange(RParen, RParen));
}
static void CheckForNullPointerDereference(Sema &S, const Expr *E) {
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=373421&r1=373420&r2=373421&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp Tue Oct 1 19:32:15 2019
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -Wpessimizing-move -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wpessimizing-move -std=c++11 -verify %s -DUSER_DEFINED
// RUN: %clang_cc1 -fsyntax-only -Wpessimizing-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
// definitions for std::move
@@ -12,7 +13,15 @@ template <class T> typename remove_refer
}
}
-struct A {};
+struct A {
+#ifdef USER_DEFINED
+ A() {}
+ A(const A &) {}
+ A(A &&) {}
+ A &operator=(const A &) { return *this; }
+ A &operator=(A &&) { return *this; }
+#endif
+};
struct B {
B() {}
B(A) {}
@@ -47,6 +56,19 @@ B test2(A a1, B b1) {
// 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 A();
+ return test1(a2);
+ return std::move(A());
+ // expected-warning at -1{{prevents copy elision}}
+ // expected-note at -2{{remove std::move call}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:""
+ return std::move(test1(a2));
+ // expected-warning at -1{{prevents copy elision}}
+ // expected-note at -2{{remove std::move call}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:29-[[@LINE-4]]:30}:""
}
A global_a;
@@ -101,11 +123,24 @@ void test6() {
// expected-note at -2{{remove std::move call}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""
// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:""
+
+ a3 = std::move(A());
+ // expected-warning at -1{{prevents copy elision}}
+ // expected-note at -2{{remove std::move call}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:8-[[@LINE-3]]:18}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:22}:""
+
A a4 = std::move(test3());
// expected-warning at -1{{prevents copy elision}}
// expected-note at -2{{remove std::move call}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""
// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:27-[[@LINE-4]]:28}:""
+
+ a4 = std::move(test3());
+ // expected-warning at -1{{prevents copy elision}}
+ // expected-note at -2{{remove std::move call}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:8-[[@LINE-3]]:18}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:26}:""
}
A test7() {
@@ -122,13 +157,13 @@ A test7() {
A a3 = (std::move(A()));
// expected-warning at -1{{prevents copy elision}}
// expected-note at -2{{remove std::move call}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:21}:""
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:26}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:21}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:25}:""
A a4 = (std::move((A())));
// expected-warning at -1{{prevents copy elision}}
// expected-note at -2{{remove std::move call}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:21}:""
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:26-[[@LINE-4]]:28}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:21}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:26-[[@LINE-4]]:27}:""
return std::move(a1);
// expected-warning at -1{{prevents copy elision}}
@@ -143,13 +178,13 @@ A test7() {
return (std::move(a1));
// expected-warning at -1{{prevents copy elision}}
// expected-note at -2{{remove std::move call}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:21}:""
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:25}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:21}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:""
return (std::move((a1)));
// expected-warning at -1{{prevents copy elision}}
// expected-note at -2{{remove std::move call}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:21}:""
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:27}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:21}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:26}:""
}
#define wrap1(x) x
@@ -227,30 +262,3 @@ namespace templates {
test2<B, A>();
}
}
-
-A init_list() {
- A a1;
- return { std::move(a1) };
- // expected-warning at -1{{prevents copy elision}}
- // expected-note at -2{{remove std::move call}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:22}:""
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:27}:""
-
- return { (std::move(a1)) };
- // expected-warning at -1{{prevents copy elision}}
- // expected-note at -2{{remove std::move call}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:23}:""
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:29}:""
-
- A a2 = { std::move(A()) };
- // expected-warning at -1{{prevents copy elision}}
- // expected-note at -2{{remove std::move call}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:22}:""
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:28}:""
-
- A a3 = { (std::move(A())) };
- // expected-warning at -1{{prevents copy elision}}
- // expected-note at -2{{remove std::move call}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:23}:""
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:26-[[@LINE-4]]:30}:""
-}
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=373421&r1=373420&r2=373421&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-redundant-move.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-redundant-move.cpp Tue Oct 1 19:32:15 2019
@@ -114,17 +114,3 @@ namespace templates {
test2<B, A>(A());
}
}
-
-A init_list(A a) {
- return { std::move(a) };
- // expected-warning at -1{{redundant move in return statement}}
- // expected-note at -2{{remove std::move call here}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:22}:""
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:26}:""
-
- return { (std::move(a)) };
- // expected-warning at -1{{redundant move in return statement}}
- // expected-note at -2{{remove std::move call here}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:23}:""
- // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:28}:""
-}
More information about the cfe-commits
mailing list