fwolff created this revision.
fwolff added reviewers: alexfh, carlosgalvezp.
fwolff added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.
fwolff requested review of this revision.
Herald added a subscriber: cfe-commits.
Fixes PR#38187. Constructors are actually already checked, but only as
functions, i.e. the check only looks at the constructor body and not at the
initializers, which misses the (common) case where constructor parameters are
moved as part of an initializer expression.
One remaining false negative is when both the move //and// the use-after-move
occur in constructor initializers. This is a lot more difficult to handle,
though, because the `bugprone-use-after-move` check is currently based on a CFG
that only takes the body into account, not the initializers, so e.g.
initialization order would have to manually be considered. I will file a
follow-up issue for this once PR#38187 is closed.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D113708
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
@@ -1338,3 +1338,26 @@
Foo Other{std::move(Bar)};
}
} // namespace UnevalContext
+
+class PR38187 {
+public:
+ PR38187(std::string val) : val_(std::move(val)) {
+ val.empty();
+ // CHECK-NOTES: [[@LINE-1]]:5: warning: 'val' used after it was moved
+ // CHECK-NOTES: [[@LINE-3]]:30: note: move occurred here
+ }
+
+private:
+ std::string val_;
+};
+
+class UseAfterMoveInCtorInit {
+public:
+ // TODO: warn here
+ UseAfterMoveInCtorInit(std::string val) : s(std::move(val)), b(val.empty()) {
+ }
+
+private:
+ std::string s;
+ bool b;
+};
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
@@ -129,8 +129,12 @@
Visited.clear();
const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
- if (!Block)
- return false;
+ if (!Block) {
+ // This can happen if MovingCall is in a constructor initializer, which is
+ // not included in the CFG because the CFG is built only from the function
+ // body.
+ Block = &TheCFG->getEntry();
+ }
return findInternal(Block, MovingCall, MovedVariable, TheUseAfterMove);
}
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
@@ -1338,3 +1338,26 @@
Foo Other{std::move(Bar)};
}
} // namespace UnevalContext
+
+class PR38187 {
+public:
+ PR38187(std::string val) : val_(std::move(val)) {
+ val.empty();
+ // CHECK-NOTES: [[@LINE-1]]:5: warning: 'val' used after it was moved
+ // CHECK-NOTES: [[@LINE-3]]:30: note: move occurred here
+ }
+
+private:
+ std::string val_;
+};
+
+class UseAfterMoveInCtorInit {
+public:
+ // TODO: warn here
+ UseAfterMoveInCtorInit(std::string val) : s(std::move(val)), b(val.empty()) {
+ }
+
+private:
+ std::string s;
+ bool b;
+};
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
@@ -129,8 +129,12 @@
Visited.clear();
const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
- if (!Block)
- return false;
+ if (!Block) {
+ // This can happen if MovingCall is in a constructor initializer, which is
+ // not included in the CFG because the CFG is built only from the function
+ // body.
+ Block = &TheCFG->getEntry();
+ }
return findInternal(Block, MovingCall, MovedVariable, TheUseAfterMove);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
