[clang-tools-extra] ddbcd98 - [clang-tidy] Correctly handle evaluation order of designated initializers.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-03-16T08:15:13Z
New Revision: ddbcd985602dcb5fe78fcf2246cf53922db1f3c3

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

LOG: [clang-tidy] Correctly handle evaluation order of designated initializers.

As designated initializers show up only in the syntactic form of the
InitListExpr, we need to make sure we're searching both forms of the
InitListExpr when determining successors in the evaluation order.

This fixes a bug in bugprone-use-after-move where previously we erroneously
concluded that two designated initializers were unsequenced. The newly added
tests fail without the fix.

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

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 beb4c44467a80..f9555be57e738 100644
--- a/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
@@ -8,6 +8,7 @@
 
 #include "ExprSequence.h"
 #include "clang/AST/ParentMapContext.h"
+#include "llvm/ADT/SmallVector.h"
 #include <optional>
 
 namespace clang::tidy::utils {
@@ -49,6 +50,7 @@ static SmallVector<const Stmt *, 1> getParentStmts(const Stmt *S,
 }
 
 namespace {
+
 bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor,
                          ASTContext *Context) {
   if (Descendant == Ancestor)
@@ -60,6 +62,17 @@ bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor,
 
   return false;
 }
+
+llvm::SmallVector<const InitListExpr *>
+getAllInitListForms(const InitListExpr *InitList) {
+  llvm::SmallVector<const InitListExpr *> result = {InitList};
+  if (const InitListExpr *AltForm = InitList->getSyntacticForm())
+    result.push_back(AltForm);
+  if (const InitListExpr *AltForm = InitList->getSemanticForm())
+    result.push_back(AltForm);
+  return result;
+}
+
 } // namespace
 
 ExprSequence::ExprSequence(const CFG *TheCFG, const Stmt *Root,
@@ -111,9 +124,12 @@ const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const {
     } else if (const auto *InitList = dyn_cast<InitListExpr>(Parent)) {
       // Initializer list: Each initializer clause is sequenced after the
       // clauses that precede it.
-      for (unsigned I = 1; I < InitList->getNumInits(); ++I) {
-        if (InitList->getInit(I - 1) == S)
-          return InitList->getInit(I);
+      for (const InitListExpr *Form : getAllInitListForms(InitList)) {
+        for (unsigned I = 1; I < Form->getNumInits(); ++I) {
+          if (Form->getInit(I - 1) == S) {
+            return Form->getInit(I);
+          }
+        }
       }
     } else if (const auto *Compound = dyn_cast<CompoundStmt>(Parent)) {
       // Compound statement: Each sub-statement is sequenced after the

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2be8bfc51d675..b53516a742e2c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -219,6 +219,10 @@ Changes in existing checks
   <clang-tidy/checks/cppcoreguidelines/slicing>` check when warning would be
   emitted in constructor for virtual base class initialization.
 
+- Improved :doc:`bugprone-use-after-move
+  <clang-tidy/checks/bugprone/use-after-move>` to understand that there is a
+  sequence point between designated initializers.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

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 281f2083857ad..45cef8abfd1f6 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
@@ -1155,18 +1155,32 @@ void initializerListSequences() {
       int i;
       A a;
     };
-    A a;
-    S1 s1{a.getInt(), std::move(a)};
+    {
+      A a;
+      S1 s1{a.getInt(), std::move(a)};
+    }
+    {
+      A a;
+      S1 s1{.i = a.getInt(), .a = std::move(a)};
+    }
   }
   {
     struct S2 {
       A a;
       int i;
     };
-    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{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
+    }
   }
 }