mboehme created this revision.
Herald added a subscriber: JDevlieghere.
I've added a test case that (without the fix) triggers the assertion,
which happens when a move happens in an implicitly called conversion
operator.
This patch also fixes nondeterministic behavior in the source code
location reported for the move when the move is constained in an init list;
this was causing buildbot failures in the previous attempt to submit
this patch (see https://reviews.llvm.org/D30569 and
https://reviews.llvm.org/rL297004).
https://reviews.llvm.org/D30650
Files:
clang-tidy/misc/UseAfterMoveCheck.cpp
test/clang-tidy/misc-use-after-move.cpp
Index: test/clang-tidy/misc-use-after-move.cpp
===================================================================
--- test/clang-tidy/misc-use-after-move.cpp
+++ test/clang-tidy/misc-use-after-move.cpp
@@ -282,7 +282,7 @@
S s{std::move(a)};
a.foo();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
- // CHECK-MESSAGES: [[@LINE-3]]:6: note: move occurred here
+ // CHECK-MESSAGES: [[@LINE-3]]:7: note: move occurred here
}
void lambdas() {
@@ -397,6 +397,21 @@
}
template void movedTypeIsDependentType<A>();
+// We handle the case correctly where the move consists of an implicit call
+// to a conversion operator.
+void implicitConversionOperator() {
+ struct Convertible {
+ operator A() && { return A(); }
+ };
+ void takeA(A a);
+
+ Convertible convertible;
+ takeA(std::move(convertible));
+ convertible;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'convertible' used after it was
moved
+ // CHECK-MESSAGES: [[@LINE-3]]:9: note: move occurred here
+}
+
// Using decltype on an expression is not a use.
void decltypeIsNotUse() {
A a;
Index: clang-tidy/misc/UseAfterMoveCheck.cpp
===================================================================
--- clang-tidy/misc/UseAfterMoveCheck.cpp
+++ clang-tidy/misc/UseAfterMoveCheck.cpp
@@ -384,6 +384,13 @@
// the direct ancestor of the std::move() that isn't one of the node
// types ignored by ignoringParenImpCasts().
stmt(forEach(expr(ignoringParenImpCasts(CallMoveMatcher))),
+ // Don't allow an InitListExpr to be the moving call. An
InitListExpr
+ // has both a syntactic and a semantic form, and the parent-child
+ // relationships are different between the two. This could cause an
+ // InitListExpr to be analyzed as the moving call in addition to the
+ // Expr that we actually want, resulting in two diagnostics with
+ // different code locations for the same move.
+ unless(initListExpr()),
unless(expr(ignoringParenImpCasts(equalsBoundNode("call-move")))))
.bind("moving-call"),
this);
@@ -398,7 +405,7 @@
const auto *MovingCall = Result.Nodes.getNodeAs<Expr>("moving-call");
const auto *Arg = Result.Nodes.getNodeAs<DeclRefExpr>("arg");
- if (!MovingCall)
+ if (!MovingCall || !MovingCall->getExprLoc().isValid())
MovingCall = CallMove;
Stmt *FunctionBody = nullptr;
Index: test/clang-tidy/misc-use-after-move.cpp
===================================================================
--- test/clang-tidy/misc-use-after-move.cpp
+++ test/clang-tidy/misc-use-after-move.cpp
@@ -282,7 +282,7 @@
S s{std::move(a)};
a.foo();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
- // CHECK-MESSAGES: [[@LINE-3]]:6: note: move occurred here
+ // CHECK-MESSAGES: [[@LINE-3]]:7: note: move occurred here
}
void lambdas() {
@@ -397,6 +397,21 @@
}
template void movedTypeIsDependentType<A>();
+// We handle the case correctly where the move consists of an implicit call
+// to a conversion operator.
+void implicitConversionOperator() {
+ struct Convertible {
+ operator A() && { return A(); }
+ };
+ void takeA(A a);
+
+ Convertible convertible;
+ takeA(std::move(convertible));
+ convertible;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'convertible' used after it was moved
+ // CHECK-MESSAGES: [[@LINE-3]]:9: note: move occurred here
+}
+
// Using decltype on an expression is not a use.
void decltypeIsNotUse() {
A a;
Index: clang-tidy/misc/UseAfterMoveCheck.cpp
===================================================================
--- clang-tidy/misc/UseAfterMoveCheck.cpp
+++ clang-tidy/misc/UseAfterMoveCheck.cpp
@@ -384,6 +384,13 @@
// the direct ancestor of the std::move() that isn't one of the node
// types ignored by ignoringParenImpCasts().
stmt(forEach(expr(ignoringParenImpCasts(CallMoveMatcher))),
+ // Don't allow an InitListExpr to be the moving call. An InitListExpr
+ // has both a syntactic and a semantic form, and the parent-child
+ // relationships are different between the two. This could cause an
+ // InitListExpr to be analyzed as the moving call in addition to the
+ // Expr that we actually want, resulting in two diagnostics with
+ // different code locations for the same move.
+ unless(initListExpr()),
unless(expr(ignoringParenImpCasts(equalsBoundNode("call-move")))))
.bind("moving-call"),
this);
@@ -398,7 +405,7 @@
const auto *MovingCall = Result.Nodes.getNodeAs<Expr>("moving-call");
const auto *Arg = Result.Nodes.getNodeAs<DeclRefExpr>("arg");
- if (!MovingCall)
+ if (!MovingCall || !MovingCall->getExprLoc().isValid())
MovingCall = CallMove;
Stmt *FunctionBody = nullptr;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
