[clang-tools-extra] r297272 - [clang-tidy] misc-use-after-move: Fix failing assertion

[Martin Bohme via cfe-commits]

Author: mboehme
Date: Wed Mar  8 06:34:51 2017
New Revision: 297272

URL: http://llvm.org/viewvc/llvm-project?rev=297272&view=rev
Log:
[clang-tidy] misc-use-after-move: Fix failing assertion

Summary:
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 D30569 and rL297004).

Reviewers: alexfh

Reviewed By: alexfh

Subscribers: Eugene.Zelenko, JDevlieghere, cfe-commits

Differential Revision: https://reviews.llvm.org/D30650

Modified:
    clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
    clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp?rev=297272&r1=297271&r2=297272&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp Wed Mar  8 06:34:51 2017
@@ -384,6 +384,13 @@ void UseAfterMoveCheck::registerMatchers
       // 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 @@ void UseAfterMoveCheck::check(const Matc
   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;

Modified: clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp?rev=297272&r1=297271&r2=297272&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp Wed Mar  8 06:34:51 2017
@@ -282,7 +282,7 @@ void moveInInitList() {
   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 @@ void movedTypeIsDependentType() {
 }
 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;