diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index c90c92b5f660a..8f4b5e8092dda 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -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 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 findInternal(const CFGBlock *Block, + const Expr *MovingCall, + const ValueDecl *MovedVariable); void getUsesAndReinits(const CFGBlock *Block, const ValueDecl *MovedVariable, llvm::SmallVectorImpl *Uses, llvm::SmallPtrSetImpl *Reinits); @@ -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 +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. @@ -111,7 +111,7 @@ bool UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall, std::unique_ptr TheCFG = CFG::buildCFG(nullptr, CodeBlock, Context, Options); if (!TheCFG) - return false; + return std::nullopt; Sequence = std::make_unique(TheCFG.get(), CodeBlock, Context); BlockMap = std::make_unique(TheCFG.get(), Context); @@ -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? @@ -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 +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). @@ -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; } } } @@ -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( @@ -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)); } }