[clang-tools-extra] c8f81ee - [clang-tidy] bugprone-use-after-move: Ctor arguments should be sequenced if ctor call is written as list-initialization.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-05-04T12:05:39Z
New Revision: c8f81ee1da325760a4bbd83c8c54ecc9645f8e20

URL: https://github.com/llvm/llvm-project/commit/c8f81ee1da325760a4bbd83c8c54ecc9645f8e20
DIFF: https://github.com/llvm/llvm-project/commit/c8f81ee1da325760a4bbd83c8c54ecc9645f8e20.diff

LOG: [clang-tidy] bugprone-use-after-move: Ctor arguments should be sequenced if ctor call is written as list-initialization.

See

https://timsong-cpp.github.io/cppwp/n4868/dcl.init#list-4

This eliminates a false positive in bugprone-use-after-move; this newly added
test used to be falsely classified as a use-after-move:

```
    A a;
    S3 s3{a.getInt(), std::move(a)};
```

Reviewed By: PiotrZSL

Differential Revision: https://reviews.llvm.org/D148110

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp b/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
index f9555be57e738..50df451ecfa26 100644
--- a/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
@@ -131,6 +131,16 @@ const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const {
           }
         }
       }
+    } else if (const auto *ConstructExpr = dyn_cast<CXXConstructExpr>(Parent)) {
+      // Constructor arguments are sequenced if the constructor call is written
+      // as list-initialization.
+      if (ConstructExpr->isListInitialization()) {
+        for (unsigned I = 1; I < ConstructExpr->getNumArgs(); ++I) {
+          if (ConstructExpr->getArg(I - 1) == S) {
+            return ConstructExpr->getArg(I);
+          }
+        }
+      }
     } else if (const auto *Compound = dyn_cast<CompoundStmt>(Parent)) {
       // Compound statement: Each sub-statement is sequenced after the
       // statements that precede it.

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e4220b3134714..98f18cc539faf 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -229,8 +229,9 @@ Changes in existing checks
   to ``std::forward``.
 
 - Improved :doc:`bugprone-use-after-move
-  <clang-tidy/checks/bugprone/use-after-move>` check to also cover constructor
-  initializers.
+  <clang-tidy/checks/bugprone/use-after-move>` check. Detect uses and moves in
+  constructor initializers. Correctly handle constructor arguments as being
+  sequenced when constructor call is written as list-initialization.
 
 - Deprecated :doc:`cert-dcl21-cpp
   <clang-tidy/checks/cert/dcl21-cpp>` check.

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
index 1e0831048dbd4..00b1da1e727e4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
@@ -1160,42 +1160,63 @@ void commaOperatorSequences() {
   }
 }
 
+namespace InitializerListSequences {
+
+struct S1 {
+  int i;
+  A a;
+};
+
+struct S2 {
+  A a;
+  int i;
+};
+
+struct S3 {
+  S3() {}
+  S3(int, A) {}
+  S3(A, int) {}
+};
+
 // An initializer list sequences its initialization clauses.
 void initializerListSequences() {
   {
-    struct S1 {
-      int i;
-      A a;
-    };
-    {
-      A a;
-      S1 s1{a.getInt(), std::move(a)};
-    }
-    {
-      A a;
-      S1 s1{.i = a.getInt(), .a = std::move(a)};
-    }
+    A a;
+    S1 s1{a.getInt(), std::move(a)};
   }
   {
-    struct S2 {
-      A a;
-      int i;
-    };
-    {
-      A a;
-      S2 s2{std::move(a), a.getInt()};
-      // CHECK-NOTES: [[@LINE-1]]:27: warning: 'a' used after it was moved
-      // CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
-    }
-    {
-      A a;
-      S2 s2{.a = std::move(a), .i = a.getInt()};
-      // CHECK-NOTES: [[@LINE-1]]:37: warning: 'a' used after it was moved
-      // CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
-    }
+    A a;
+    S1 s1{.i = a.getInt(), .a = std::move(a)};
+  }
+  {
+    A a;
+    S2 s2{std::move(a), a.getInt()};
+    // CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
+    // CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
+  }
+  {
+    A a;
+    S2 s2{.a = std::move(a), .i = a.getInt()};
+    // CHECK-NOTES: [[@LINE-1]]:35: warning: 'a' used after it was moved
+    // CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
+  }
+  {
+    // Check the case where the constructed type has a constructor and the
+    // initializer list therefore manifests as a `CXXConstructExpr` instead of
+    // an `InitListExpr`.
+    A a;
+    S3 s3{a.getInt(), std::move(a)};
+  }
+  {
+    A a;
+    S3 s3{std::move(a), a.getInt()};
+    // CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
+    // CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
   }
 }
 
+} // namespace InitializerListSequences
+
 // A declaration statement containing multiple declarations sequences the
 // initializer expressions.
 void declarationSequences() {