[PATCH] D145581: [clang-tidy] In C++17, callee is guaranteed to be sequenced before arguments.

Martin Böhme via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 8 05:06:11 PST 2023


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;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D145581.503330.patch
Type: text/x-patch
Size: 2430 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230308/669d1218/attachment.bin>


More information about the cfe-commits mailing list