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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
