[clang][dataflow] Support CXXParenListInitExpr in PropagateResultObject().

[martinboehme]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-analysis

Author: None (martinboehme)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/89235.diff

4 Files Affected:

• (modified) clang/include/clang/Analysis/FlowSensitive/ASTOps.h (+4)
• (modified) clang/lib/Analysis/FlowSensitive/ASTOps.cpp (+25-10)
• (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+30-19)
• (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+73)

diff --git a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
index 27ad32c1694f77..f9fd3db1fb67fc 100644
--- a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
@@ -56,6 +56,7 @@ class RecordInitListHelper {
 public:
   // `InitList` must have record type.
   RecordInitListHelper(const InitListExpr *InitList);
+  RecordInitListHelper(const CXXParenListInitExpr *ParenInitList);
 
   // Base classes with their associated initializer expressions.
   ArrayRef<std::pair<const CXXBaseSpecifier *, Expr *>> base_inits() const {
@@ -68,6 +69,9 @@ class RecordInitListHelper {
   }
 
 private:
+  RecordInitListHelper(QualType Ty, std::vector<const FieldDecl *> Fields,
+                       ArrayRef<Expr *> Inits);
+
   SmallVector<std::pair<const CXXBaseSpecifier *, Expr *>> BaseInits;
   SmallVector<std::pair<const FieldDecl *, Expr *>> FieldInits;
 
diff --git a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp
index 75188aef4d1a43..683f251b0b4fff 100644
--- a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp
@@ -80,11 +80,12 @@ bool containsSameFields(const FieldSet &Fields,
 }
 
 /// Returns the fields of a `RecordDecl` that are initialized by an
-/// `InitListExpr`, in the order in which they appear in
-/// `InitListExpr::inits()`.
-/// `Init->getType()` must be a record type.
+/// `InitListExpr` or `CXXParenListInitExpr`, in the order in which they appear
+/// in `InitListExpr::inits()` / `CXXParenListInitExpr::getInitExprs()`.
+/// `InitList->getType()` must be a record type.
+template <class InitListT>
 static std::vector<const FieldDecl *>
-getFieldsForInitListExpr(const InitListExpr *InitList) {
+getFieldsForInitListExpr(const InitListT *InitList) {
   const RecordDecl *RD = InitList->getType()->getAsRecordDecl();
   assert(RD != nullptr);
 
@@ -105,19 +106,29 @@ getFieldsForInitListExpr(const InitListExpr *InitList) {
   return Fields;
 }
 
-RecordInitListHelper::RecordInitListHelper(const InitListExpr *InitList) {
-  auto *RD = InitList->getType()->getAsCXXRecordDecl();
-  assert(RD != nullptr);
+RecordInitListHelper::RecordInitListHelper(const InitListExpr *InitList)
+    : RecordInitListHelper(InitList->getType(),
+                           getFieldsForInitListExpr(InitList),
+                           InitList->inits()) {}
+
+RecordInitListHelper::RecordInitListHelper(
+    const CXXParenListInitExpr *ParenInitList)
+    : RecordInitListHelper(ParenInitList->getType(),
+                           getFieldsForInitListExpr(ParenInitList),
+                           ParenInitList->getInitExprs()) {}
 
-  std::vector<const FieldDecl *> Fields = getFieldsForInitListExpr(InitList);
-  ArrayRef<Expr *> Inits = InitList->inits();
+RecordInitListHelper::RecordInitListHelper(
+    QualType Ty, std::vector<const FieldDecl *> Fields,
+    ArrayRef<Expr *> Inits) {
+  auto *RD = Ty->getAsCXXRecordDecl();
+  assert(RD != nullptr);
 
   // Unions initialized with an empty initializer list need special treatment.
   // For structs/classes initialized with an empty initializer list, Clang
   // puts `ImplicitValueInitExpr`s in `InitListExpr::inits()`, but for unions,
   // it doesn't do this -- so we create an `ImplicitValueInitExpr` ourselves.
   SmallVector<Expr *> InitsForUnion;
-  if (InitList->getType()->isUnionType() && Inits.empty()) {
+  if (Ty->isUnionType() && Inits.empty()) {
     assert(Fields.size() == 1);
     ImplicitValueInitForUnion.emplace(Fields.front()->getType());
     InitsForUnion.push_back(&*ImplicitValueInitForUnion);
@@ -217,6 +228,10 @@ static void getReferencedDecls(const Stmt &S, ReferencedDecls &Referenced) {
     if (InitList->getType()->isRecordType())
       for (const auto *FD : getFieldsForInitListExpr(InitList))
         Referenced.Fields.insert(FD);
+  } else if (auto *ParenInitList = dyn_cast<CXXParenListInitExpr>(&S)) {
+    if (ParenInitList->getType()->isRecordType())
+      for (const auto *FD : getFieldsForInitListExpr(ParenInitList))
+        Referenced.Fields.insert(FD);
   }
 }
 
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 3f1600d9ac5d87..cbc6f7e159f757 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -401,6 +401,29 @@ class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
     return true;
   }
 
+  void
+  PropagateResultObjectToRecordInitList(const RecordInitListHelper &InitList,
+                                        RecordStorageLocation *Loc) {
+    for (auto [Base, Init] : InitList.base_inits()) {
+      assert(Base->getType().getCanonicalType() ==
+             Init->getType().getCanonicalType());
+
+      // Storage location for the base class is the same as that of the
+      // derived class because we "flatten" the object hierarchy and put all
+      // fields in `RecordStorageLocation` of the derived class.
+      PropagateResultObject(Init, Loc);
+    }
+
+    for (auto [Field, Init] : InitList.field_inits()) {
+      // Fields of non-record type are handled in
+      // `TransferVisitor::VisitInitListExpr()`.
+      if (!Field->getType()->isRecordType())
+        continue;
+      PropagateResultObject(Init,
+                            cast<RecordStorageLocation>(Loc->getChild(*Field)));
+    }
+  }
+
   // Assigns `Loc` as the result object location of `E`, then propagates the
   // location to all lower-level prvalues that initialize the same object as
   // `E` (or one of its base classes or member variables).
@@ -440,26 +463,14 @@ class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
         return;
       }
 
-      RecordInitListHelper InitListHelper(InitList);
-
-      for (auto [Base, Init] : InitListHelper.base_inits()) {
-        assert(Base->getType().getCanonicalType() ==
-               Init->getType().getCanonicalType());
-
-        // Storage location for the base class is the same as that of the
-        // derived class because we "flatten" the object hierarchy and put all
-        // fields in `RecordStorageLocation` of the derived class.
-        PropagateResultObject(Init, Loc);
-      }
+      PropagateResultObjectToRecordInitList(RecordInitListHelper(InitList),
+                                            Loc);
+      return;
+    }
 
-      for (auto [Field, Init] : InitListHelper.field_inits()) {
-        // Fields of non-record type are handled in
-        // `TransferVisitor::VisitInitListExpr()`.
-        if (!Field->getType()->isRecordType())
-          continue;
-        PropagateResultObject(
-            Init, cast<RecordStorageLocation>(Loc->getChild(*Field)));
-      }
+    if (auto *ParenInitList = dyn_cast<CXXParenListInitExpr>(E)) {
+      PropagateResultObjectToRecordInitList(RecordInitListHelper(ParenInitList),
+                                            Loc);
       return;
     }
 
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 97ec32126c1dc4..01b8fd0df23519 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -3098,6 +3098,79 @@ TEST(TransferTest, ResultObjectLocationForCXXOperatorCallExpr) {
       });
 }
 
+TEST(TransferTest, ResultObjectLocationForInitListExpr) {
+  std::string Code = R"cc(
+    struct Inner {};
+
+    struct Outer { Inner I; };
+
+    void target() {
+      Outer O = { Inner() };
+      // [[p]]
+    }
+  )cc";
+  using ast_matchers::asString;
+  using ast_matchers::cxxConstructExpr;
+  using ast_matchers::hasType;
+  using ast_matchers::match;
+  using ast_matchers::selectFirst;
+  using ast_matchers::traverse;
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+        auto *Construct = selectFirst<CXXConstructExpr>(
+            "construct",
+            match(
+                cxxConstructExpr(hasType(asString("Inner"))).bind("construct"),
+                ASTCtx));
+
+        EXPECT_EQ(
+            &Env.getResultObjectLocation(*Construct),
+            &getFieldLoc(getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "O"),
+                         "I", ASTCtx));
+      });
+}
+
+TEST(TransferTest, ResultObjectLocationForParenInitListExpr) {
+  std::string Code = R"cc(
+    struct Inner {};
+
+    struct Outer { Inner I; };
+
+    void target() {
+      Outer O((Inner()));
+      // [[p]]
+    }
+  )cc";
+  using ast_matchers::asString;
+  using ast_matchers::cxxConstructExpr;
+  using ast_matchers::hasType;
+  using ast_matchers::match;
+  using ast_matchers::selectFirst;
+  using ast_matchers::traverse;
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+        auto *Construct = selectFirst<CXXConstructExpr>(
+            "construct",
+            match(
+                cxxConstructExpr(hasType(asString("Inner"))).bind("construct"),
+                ASTCtx));
+
+        EXPECT_EQ(
+            &Env.getResultObjectLocation(*Construct),
+            &getFieldLoc(getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "O"),
+                         "I", ASTCtx));
+      },
+      LangStandard::lang_cxx20);
+}
+
 // Check that the `std::strong_ordering` object returned by builtin `<=>` has a
 // correctly modeled result object location.
 TEST(TransferTest, ResultObjectLocationForBuiltinSpaceshipOperator) {

[ymand]

Suggested change

	if (!Field->getType()->isRecordType())
	continue;
	PropagateResultObject(Init,
	cast<RecordStorageLocation>(Loc->getChild(*Field)));
	if (Field->getType()->isRecordType())
	PropagateResultObject(Init,
	cast<RecordStorageLocation>(Loc->getChild(*Field)));

[martinboehme]

Good point -- done.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 676d3bafc09d0c331a04b813804407334de12917 5a00d607ca863e365292db154fb1d8b7206bf66a -- clang/include/clang/Analysis/FlowSensitive/ASTOps.h clang/lib/Analysis/FlowSensitive/ASTOps.cpp clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

View the diff from clang-format here.

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 138773460d..4c6f9cd671 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -418,8 +418,8 @@ public:
       // Fields of non-record type are handled in
       // `TransferVisitor::VisitInitListExpr()`.
       if (Field->getType()->isRecordType())
-        PropagateResultObject(Init,
-                              cast<RecordStorageLocation>(Loc->getChild(*Field)));
+        PropagateResultObject(
+            Init, cast<RecordStorageLocation>(Loc->getChild(*Field)));
     }
   }