[clang-tidy] Add additional diagnostic to misc-use-after-move

Summary:
This adds a diagnostic to the misc-use-after-move check that is output when the
use happens on a later loop iteration than the move, for example:

    A a;
    for (int i = 0; i < 10; ++i) {
      a.foo();
      std::move(a);
    }

This situation can be confusing to users because, in terms of source code
location, the use is above the move. This can make it look as if the warning
is a false positive, particularly if the loop is long but the use and move are
close together.

In cases like these, misc-use-after-move will now output an additional
diagnostic:

  a.cpp:393:7: note: the use happens in a later loop iteration than the move

Reviewers: hokein

Subscribers: cfe-commits

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

llvm-svn: 284235
This commit is contained in:
Martin Bohme 2016-10-14 13:23:39 +00:00
parent 270173f2db
commit 934743f8f8
2 changed files with 18 additions and 12 deletions

View File

@ -562,18 +562,24 @@ void UseAfterMoveFinder::getReinits(
} }
static void emitDiagnostic(const Expr *MovingCall, static void emitDiagnostic(const Expr *MovingCall,
const ValueDecl *MovedVariable, const DeclRefExpr *MoveArg,
const UseAfterMove &Use, ClangTidyCheck *Check, const UseAfterMove &Use, ClangTidyCheck *Check,
ASTContext *Context) { ASTContext *Context) {
Check->diag(Use.DeclRef->getExprLoc(), "'%0' used after it was moved") SourceLocation UseLoc = Use.DeclRef->getExprLoc();
<< MovedVariable->getName(); SourceLocation MoveLoc = MovingCall->getExprLoc();
Check->diag(MovingCall->getExprLoc(), "move occurred here",
DiagnosticIDs::Note); Check->diag(UseLoc, "'%0' used after it was moved")
<< MoveArg->getDecl()->getName();
Check->diag(MoveLoc, "move occurred here", DiagnosticIDs::Note);
if (Use.EvaluationOrderUndefined) { if (Use.EvaluationOrderUndefined) {
Check->diag(Use.DeclRef->getExprLoc(), Check->diag(UseLoc,
"the use and move are unsequenced, i.e. there is no guarantee " "the use and move are unsequenced, i.e. there is no guarantee "
"about the order in which they are evaluated", "about the order in which they are evaluated",
DiagnosticIDs::Note); DiagnosticIDs::Note);
} else if (UseLoc < MoveLoc || Use.DeclRef == MoveArg) {
Check->diag(UseLoc,
"the use happens in a later loop iteration than the move",
DiagnosticIDs::Note);
} }
} }
@ -625,8 +631,6 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) {
else else
return; return;
const ValueDecl *MovedVariable = Arg->getDecl();
// Ignore the std::move if the variable that was passed to it isn't a local // Ignore the std::move if the variable that was passed to it isn't a local
// variable. // variable.
if (!Arg->getDecl()->getDeclContext()->isFunctionOrMethod()) if (!Arg->getDecl()->getDeclContext()->isFunctionOrMethod())
@ -634,8 +638,8 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) {
UseAfterMoveFinder finder(Result.Context); UseAfterMoveFinder finder(Result.Context);
UseAfterMove Use; UseAfterMove Use;
if (finder.find(FunctionBody, MovingCall, MovedVariable, &Use)) if (finder.find(FunctionBody, MovingCall, Arg->getDecl(), &Use))
emitDiagnostic(MovingCall, MovedVariable, Use, this, Result.Context); emitDiagnostic(MovingCall, Arg, Use, this, Result.Context);
} }
} // namespace misc } // namespace misc

View File

@ -133,6 +133,7 @@ void moveAfterMove() {
std::move(a); std::move(a);
// CHECK-MESSAGES: [[@LINE-1]]:17: warning: 'a' used after it was moved // CHECK-MESSAGES: [[@LINE-1]]:17: warning: 'a' used after it was moved
// CHECK-MESSAGES: [[@LINE-2]]:7: note: move occurred here // CHECK-MESSAGES: [[@LINE-2]]:7: note: move occurred here
// CHECK-MESSAGES: [[@LINE-3]]:17: note: the use happens in a later loop
} }
} }
} }
@ -391,7 +392,8 @@ void useAndMoveInLoop() {
for (int i = 0; i < 10; ++i) { for (int i = 0; i < 10; ++i) {
a.foo(); a.foo();
// CHECK-MESSAGES: [[@LINE-1]]:7: warning: 'a' used after it was moved // CHECK-MESSAGES: [[@LINE-1]]:7: warning: 'a' used after it was moved
// CHECK-MESSAGES: [[@LINE+1]]:7: note: move occurred here // CHECK-MESSAGES: [[@LINE+2]]:7: note: move occurred here
// CHECK-MESSAGES: [[@LINE-3]]:7: note: the use happens in a later loop
std::move(a); std::move(a);
} }
} }
@ -586,7 +588,7 @@ void assignments(int i) {
} }
} }
// Passing the object to a function through a non-const pointer or refernce // Passing the object to a function through a non-const pointer or reference
// counts as a re-initialization. // counts as a re-initialization.
void passByNonConstPointer(A *); void passByNonConstPointer(A *);
void passByNonConstReference(A &); void passByNonConstReference(A &);