MarcoFalke created this revision.
MarcoFalke added a project: clang-tools-extra.
Herald added subscribers: PiotrZSL, carlosgalvezp.
Herald added a reviewer: njames93.
Herald added a project: All.
MarcoFalke requested review of this revision.
Herald added a subscriber: cfe-commits.
Fixes https://github.com/llvm/llvm-project/issues/51844
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D146288
Files:
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
@@ -1353,6 +1353,70 @@
}
} // namespace UnevalContext
+class CtorInit {
+public:
+ CtorInit(std::string val)
+ : a{val.empty()}, // fine
+ s{std::move(val)},
+ b{val.empty()}
+ // CHECK-NOTES: [[@LINE-1]]:11: warning: 'val' used after it was moved
+ // CHECK-NOTES: [[@LINE-3]]:9: note: move occurred here
+ {}
+
+private:
+ bool a;
+ std::string s;
+ bool b;
+};
+
+class CtorInitLambda {
+public:
+ CtorInitLambda(std::string val)
+ : a{val.empty()}, // fine
+ s{std::move(val)},
+ b{[&] { return val.empty(); }()},
+ // CHECK-NOTES: [[@LINE-1]]:12: warning: 'val' used after it was moved
+ // CHECK-NOTES: [[@LINE-3]]:9: note: move occurred here
+ c{[] {
+ std::string str{};
+ std::move(str);
+ return str.empty();
+ // CHECK-NOTES: [[@LINE-1]]:18: warning: 'str' used after it was moved
+ // CHECK-NOTES: [[@LINE-3]]:11: note: move occurred here
+ }()} {
+ std::move(val);
+ // CHECK-NOTES: [[@LINE-1]]:15: warning: 'val' used after it was moved
+ // CHECK-NOTES: [[@LINE-13]]:9: note: move occurred here
+ std::string val2{};
+ std::move(val2);
+ val2.empty();
+ // CHECK-NOTES: [[@LINE-1]]:5: warning: 'val2' used after it was moved
+ // CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
+ }
+
+private:
+ bool a;
+ std::string s;
+ bool b;
+ bool c;
+};
+
+class CtorInitOrder {
+public:
+ CtorInitOrder(std::string val)
+ : a{val.empty()}, // fine
+ b{val.empty()},
+ // CHECK-NOTES: [[@LINE-1]]:11: warning: 'val' used after it was moved
+ s{std::move(val)} {} // wrong order
+ // CHECK-NOTES: [[@LINE-1]]:9: note: move occurred here
+ // CHECK-NOTES: [[@LINE-4]]:11: note: the use happens in a later loop iteration than the move
+
+private:
+ bool a;
+ std::string s;
+ bool b;
+};
+
class PR38187 {
public:
PR38187(std::string val) : val_(std::move(val)) {
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -58,11 +58,11 @@
public:
UseAfterMoveFinder(ASTContext *TheContext);
- // Within the given function body, finds the first use of 'MovedVariable' that
+ // Within the given code block, finds the first use of 'MovedVariable' that
// 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 *FunctionBody, const Expr *MovingCall,
+ bool find(Stmt *CodeBlock, const Expr *MovingCall,
const ValueDecl *MovedVariable, UseAfterMove *TheUseAfterMove);
private:
@@ -104,7 +104,7 @@
UseAfterMoveFinder::UseAfterMoveFinder(ASTContext *TheContext)
: Context(TheContext) {}
-bool UseAfterMoveFinder::find(Stmt *FunctionBody, const Expr *MovingCall,
+bool UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall,
const ValueDecl *MovedVariable,
UseAfterMove *TheUseAfterMove) {
// Generate the CFG manually instead of through an AnalysisDeclContext because
@@ -118,12 +118,11 @@
Options.AddImplicitDtors = true;
Options.AddTemporaryDtors = true;
std::unique_ptr<CFG> TheCFG =
- CFG::buildCFG(nullptr, FunctionBody, Context, Options);
+ CFG::buildCFG(nullptr, CodeBlock, Context, Options);
if (!TheCFG)
return false;
- Sequence =
- std::make_unique<ExprSequence>(TheCFG.get(), FunctionBody, Context);
+ Sequence = std::make_unique<ExprSequence>(TheCFG.get(), CodeBlock, Context);
BlockMap = std::make_unique<StmtToBlockMap>(TheCFG.get(), Context);
Visited.clear();
@@ -401,7 +400,16 @@
auto CallMoveMatcher =
callExpr(callee(functionDecl(hasName("::std::move"))), argumentCountIs(1),
hasArgument(0, declRefExpr().bind("arg")),
- anyOf(hasAncestor(compoundStmt(
+ anyOf(hasAncestor(
+ stmt(
+ // For some reason cxxCtorInitializer didn't
+ // match, so work around by matching the topmost
+ // Stmt inside the CXXCtorInitializer.
+ hasParent(cxxConstructorDecl().bind("containing-ctor")),
+ // Exclude the constructor body
+ unless(compoundStmt()))
+ .bind("containing-ctor-init-stmt")),
+ hasAncestor(compoundStmt(
hasParent(lambdaExpr().bind("containing-lambda")))),
hasAncestor(functionDecl().bind("containing-func"))),
unless(inDecltypeOrTemplateArg()),
@@ -434,6 +442,10 @@
}
void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *ContainingCtor =
+ Result.Nodes.getNodeAs<CXXConstructorDecl>("containing-ctor");
+ const auto *ContainingCtorInitStmt =
+ Result.Nodes.getNodeAs<Stmt>("containing-ctor-init-stmt");
const auto *ContainingLambda =
Result.Nodes.getNodeAs<LambdaExpr>("containing-lambda");
const auto *ContainingFunc =
@@ -445,23 +457,47 @@
if (!MovingCall || !MovingCall->getExprLoc().isValid())
MovingCall = CallMove;
- Stmt *FunctionBody = nullptr;
- if (ContainingLambda)
- FunctionBody = ContainingLambda->getBody();
- else if (ContainingFunc)
- FunctionBody = ContainingFunc->getBody();
- else
- return;
-
// Ignore the std::move if the variable that was passed to it isn't a local
// variable.
if (!Arg->getDecl()->getDeclContext()->isFunctionOrMethod())
return;
- UseAfterMoveFinder Finder(Result.Context);
- UseAfterMove Use;
- if (Finder.find(FunctionBody, MovingCall, Arg->getDecl(), &Use))
- emitDiagnostic(MovingCall, Arg, Use, this, Result.Context);
+ // Collect all code blocks that could use the arg after move.
+ llvm::SmallVector<Stmt *> CodeBlocks{};
+ if (ContainingCtor) {
+ CodeBlocks.push_back(ContainingCtor->getBody());
+ if (ContainingCtorInitStmt) {
+ // Find the constructor initializer's member field.
+ FieldDecl *ContainingCtorField{nullptr};
+ for (CXXCtorInitializer *Init : ContainingCtor->inits()) {
+ if (Init->getInit() == ContainingCtorInitStmt) {
+ ContainingCtorField = Init->getMember();
+ break;
+ }
+ }
+ if (ContainingCtorField) {
+ // Collect the constructor initializer expressions.
+ for (CXXCtorInitializer *Init : ContainingCtor->inits()) {
+ if (Init->getMember() && Init->getMember()->getFieldIndex() >=
+ ContainingCtorField->getFieldIndex()) {
+ if (Init->getInit())
+ CodeBlocks.push_back(Init->getInit());
+ }
+ }
+ }
+ }
+ } else if (ContainingLambda)
+ CodeBlocks.push_back(ContainingLambda->getBody());
+ else if (ContainingFunc) {
+ CodeBlocks.push_back(ContainingFunc->getBody());
+ }
+
+ for (Stmt *CodeBlock : CodeBlocks) {
+ UseAfterMoveFinder Finder(Result.Context);
+ UseAfterMove Use;
+ if (Finder.find(CodeBlock, MovingCall, Arg->getDecl(), &Use))
+ emitDiagnostic(MovingCall, Arg, Use, this, Result.Context);
+ }
}
} // namespace clang::tidy::bugprone
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
