Skip to content

[clang-tidy] let UseAfterMoveFinder::find() return an optional<UseAfterMove> #98100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 29 additions & 27 deletions clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,13 @@ class UseAfterMoveFinder {
// occurs after 'MovingCall' (the expression that performs the move). If a
// use-after-move is found, writes information about it to 'TheUseAfterMove'.
// Returns whether a use-after-move was found.
bool find(Stmt *CodeBlock, const Expr *MovingCall,
const DeclRefExpr *MovedVariable, UseAfterMove *TheUseAfterMove);
std::optional<UseAfterMove> find(Stmt *CodeBlock, const Expr *MovingCall,
const DeclRefExpr *MovedVariable);

private:
bool findInternal(const CFGBlock *Block, const Expr *MovingCall,
const ValueDecl *MovedVariable,
UseAfterMove *TheUseAfterMove);
std::optional<UseAfterMove> findInternal(const CFGBlock *Block,
const Expr *MovingCall,
const ValueDecl *MovedVariable);
void getUsesAndReinits(const CFGBlock *Block, const ValueDecl *MovedVariable,
llvm::SmallVectorImpl<const DeclRefExpr *> *Uses,
llvm::SmallPtrSetImpl<const Stmt *> *Reinits);
Expand Down Expand Up @@ -95,9 +95,9 @@ static StatementMatcher inDecltypeOrTemplateArg() {
UseAfterMoveFinder::UseAfterMoveFinder(ASTContext *TheContext)
: Context(TheContext) {}

bool UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall,
const DeclRefExpr *MovedVariable,
UseAfterMove *TheUseAfterMove) {
std::optional<UseAfterMove>
UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall,
const DeclRefExpr *MovedVariable) {
// Generate the CFG manually instead of through an AnalysisDeclContext because
// it seems the latter can't be used to generate a CFG for the body of a
// lambda.
Expand All @@ -111,7 +111,7 @@ bool UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall,
std::unique_ptr<CFG> TheCFG =
CFG::buildCFG(nullptr, CodeBlock, Context, Options);
if (!TheCFG)
return false;
return std::nullopt;

Sequence = std::make_unique<ExprSequence>(TheCFG.get(), CodeBlock, Context);
BlockMap = std::make_unique<StmtToBlockMap>(TheCFG.get(), Context);
Expand All @@ -125,10 +125,10 @@ bool UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall,
MoveBlock = &TheCFG->getEntry();
}

bool Found = findInternal(MoveBlock, MovingCall, MovedVariable->getDecl(),
TheUseAfterMove);
auto TheUseAfterMove =
findInternal(MoveBlock, MovingCall, MovedVariable->getDecl());

if (Found) {
if (TheUseAfterMove) {
if (const CFGBlock *UseBlock =
BlockMap->blockContainingStmt(TheUseAfterMove->DeclRef)) {
// Does the use happen in a later loop iteration than the move?
Expand All @@ -142,15 +142,14 @@ bool UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall,
: CFA.isReachable(UseBlock, MoveBlock);
}
}
return Found;
return TheUseAfterMove;
}

bool UseAfterMoveFinder::findInternal(const CFGBlock *Block,
const Expr *MovingCall,
const ValueDecl *MovedVariable,
UseAfterMove *TheUseAfterMove) {
std::optional<UseAfterMove>
UseAfterMoveFinder::findInternal(const CFGBlock *Block, const Expr *MovingCall,
const ValueDecl *MovedVariable) {
if (Visited.count(Block))
return false;
return std::nullopt;

// Mark the block as visited (except if this is the block containing the
// std::move() and it's being visited the first time).
Expand Down Expand Up @@ -189,17 +188,18 @@ bool UseAfterMoveFinder::findInternal(const CFGBlock *Block,
}

if (!HaveSavingReinit) {
TheUseAfterMove->DeclRef = Use;
UseAfterMove TheUseAfterMove;
TheUseAfterMove.DeclRef = Use;

// Is this a use-after-move that depends on order of evaluation?
// This is the case if the move potentially comes after the use (and we
// already know that use potentially comes after the move, which taken
// together tells us that the ordering is unclear).
TheUseAfterMove->EvaluationOrderUndefined =
TheUseAfterMove.EvaluationOrderUndefined =
MovingCall != nullptr &&
Sequence->potentiallyAfter(MovingCall, Use);

return true;
return TheUseAfterMove;
}
}
}
Expand All @@ -208,12 +208,15 @@ bool UseAfterMoveFinder::findInternal(const CFGBlock *Block,
// successors.
if (Reinits.empty()) {
for (const auto &Succ : Block->succs()) {
if (Succ && findInternal(Succ, nullptr, MovedVariable, TheUseAfterMove))
return true;
if (Succ) {
if (auto Found = findInternal(Succ, nullptr, MovedVariable)) {
return Found;
}
}
}
}

return false;
return std::nullopt;
}

void UseAfterMoveFinder::getUsesAndReinits(
Expand Down Expand Up @@ -518,9 +521,8 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) {

for (Stmt *CodeBlock : CodeBlocks) {
UseAfterMoveFinder Finder(Result.Context);
UseAfterMove Use;
if (Finder.find(CodeBlock, MovingCall, Arg, &Use))
emitDiagnostic(MovingCall, Arg, Use, this, Result.Context,
if (auto Use = Finder.find(CodeBlock, MovingCall, Arg))
emitDiagnostic(MovingCall, Arg, *Use, this, Result.Context,
determineMoveType(MoveDecl));
}
}
Expand Down
Loading