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

[Martin Böhme via Phabricator via cfe-commits]

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.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145906

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
@@ -1155,18 +1155,32 @@
       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
+    }
   }
 }
 
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
@@ -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 @@
 }
 
 namespace {
+
 bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor,
                          ASTContext *Context) {
   if (Descendant == Ancestor)
@@ -60,6 +62,17 @@
 
   return false;
 }
+
+llvm::SmallVector<const InitListExpr *>
+allInitListExprForms(const InitListExpr *InitList) {
+  llvm::SmallVector<const InitListExpr *> result = {InitList};
+  if (InitList->isSemanticForm() && InitList->getSyntacticForm())
+    result.push_back(InitList->getSyntacticForm());
+  if (InitList->isSyntacticForm() && InitList->getSemanticForm())
+    result.push_back(InitList->getSemanticForm());
+  return result;
+}
+
 } // namespace
 
 ExprSequence::ExprSequence(const CFG *TheCFG, const Stmt *Root,
@@ -111,9 +124,12 @@
     } 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 : allInitListExprForms(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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D145906.504551.patch
Type: text/x-patch
Size: 3120 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230313/988d3887/attachment-0001.bin>