mboehme created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
This eliminates false positives in bugprone-use-after-move where a variable
is used in the callee and moved from in the arguments.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D145581
Files:
clang-tools-extra/clang-tidy/utils/ExprSequence.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
@@ -1293,6 +1293,18 @@
}
}
+// In a function call, the expression that determines the callee is sequenced
+// before the arguments.
+namespace CalleeSequencedBeforeArguments {
+struct A {
+ void foo(std::unique_ptr<A>) {}
+};
+void calleeSequencedBeforeArguments() {
+ std::unique_ptr<A> a;
+ a->foo(std::move(a));
+}
+} // namespace CalleeSequencedBeforeArguments
+
// Some statements in templates (e.g. null, break and continue statements) may
// be shared between the uninstantiated and instantiated versions of the
// template and therefore have multiple parents. Make sure the sequencing code
@@ -1363,4 +1375,4 @@
private:
std::string val_;
-};
+};
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
@@ -82,9 +82,29 @@
return true;
}
+ SmallVector<const Stmt *, 1> BeforeParents = getParentStmts(Before, Context);
+
+ // Since C++17, the callee of a call expression is guaranteed to be sequenced
+ // before all of the arguments.
+ // We handle this as a special case rather than using the general
+ // `getSequenceSuccessor` logic above because the callee expression doesn't
+ // have an unambiguous successor; the order in which arguments are evaluated
+ // is indeterminate.
+ if (Context->getLangOpts().CPlusPlus17) {
+ for (const Stmt *Parent : BeforeParents) {
+ if (const auto *call = dyn_cast<CallExpr>(Parent);
+ call && call->getCallee() == Before) {
+ for (const Expr *arg : call->arguments()) {
+ if (isDescendantOrEqual(After, arg, Context))
+ return true;
+ }
+ }
+ }
+ }
+
// If 'After' is a parent of 'Before' or is sequenced after one of these
// parents, we know that it is sequenced after 'Before'.
- for (const Stmt *Parent : getParentStmts(Before, Context)) {
+ for (const Stmt *Parent : BeforeParents) {
if (Parent == After || inSequence(Parent, After))
return true;
}
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
@@ -1293,6 +1293,18 @@
}
}
+// In a function call, the expression that determines the callee is sequenced
+// before the arguments.
+namespace CalleeSequencedBeforeArguments {
+struct A {
+ void foo(std::unique_ptr<A>) {}
+};
+void calleeSequencedBeforeArguments() {
+ std::unique_ptr<A> a;
+ a->foo(std::move(a));
+}
+} // namespace CalleeSequencedBeforeArguments
+
// Some statements in templates (e.g. null, break and continue statements) may
// be shared between the uninstantiated and instantiated versions of the
// template and therefore have multiple parents. Make sure the sequencing code
@@ -1363,4 +1375,4 @@
private:
std::string val_;
-};
+};
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
@@ -82,9 +82,29 @@
return true;
}
+ SmallVector<const Stmt *, 1> BeforeParents = getParentStmts(Before, Context);
+
+ // Since C++17, the callee of a call expression is guaranteed to be sequenced
+ // before all of the arguments.
+ // We handle this as a special case rather than using the general
+ // `getSequenceSuccessor` logic above because the callee expression doesn't
+ // have an unambiguous successor; the order in which arguments are evaluated
+ // is indeterminate.
+ if (Context->getLangOpts().CPlusPlus17) {
+ for (const Stmt *Parent : BeforeParents) {
+ if (const auto *call = dyn_cast<CallExpr>(Parent);
+ call && call->getCallee() == Before) {
+ for (const Expr *arg : call->arguments()) {
+ if (isDescendantOrEqual(After, arg, Context))
+ return true;
+ }
+ }
+ }
+ }
+
// If 'After' is a parent of 'Before' or is sequenced after one of these
// parents, we know that it is sequenced after 'Before'.
- for (const Stmt *Parent : getParentStmts(Before, Context)) {
+ for (const Stmt *Parent : BeforeParents) {
if (Parent == After || inSequence(Parent, After))
return true;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
