[clang][dataflow] Include fields initialized in an `InitListExpr` in `getModeledFields()`.

[mboehme]

Summary

Previously, we were including these fields only in the specific instance that was initialized by the InitListExpr, but not in other instances of the same type. This is inconsistent and error-prone.

Depends On D154952

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

 getImplicitObjectLocation(const CXXMemberCallExpr &MCE, const Environment &Env);
 
 /// Returns the storage location for the base object of a `MemberExpr`, or null
 /// if none is defined in the environment. Dereferences the pointer if the
 /// member expression was written using `->`.
 AggregateStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
                                                 const Environment &Env);
 
+/// Returns the fields of `RD` that are initialized by an `InitListExpr`, in the
+/// order in which they appear in `InitListExpr::inits()`.
+std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD);
+
 } // namespace dataflow
 } // namespace clang
 
 #endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_DATAFLOWENVIRONMENT_H

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

if (auto *DS = dyn_cast<DeclStmt>(&S)) {
     insertIfFunction(*E->getDecl(), Funcs);
   } else if (auto *E = dyn_cast<MemberExpr>(&S)) {
     // FIXME: should we be using `E->getFoundDecl()`?
     const ValueDecl *VD = E->getMemberDecl();
     insertIfGlobal(*VD, Vars);
     insertIfFunction(*VD, Funcs);
     if (const auto *FD = dyn_cast<FieldDecl>(VD))
       Fields.insert(FD);
+  } else if (auto *InitList = dyn_cast<InitListExpr>(&S)) {
+    if (RecordDecl *RD = InitList->getType()->getAsRecordDecl())
+      for (const auto *FD : getFieldsForInitListExpr(RD))
+        Fields.insert(FD);
   }
 }
 
 // FIXME: Add support for resetting globals after function calls to enable
 // the implementation of sound analyses.
 void Environment::initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl) {
   assert(FuncDecl->getBody() != nullptr);
 
AggregateStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
   if (ME.isArrow()) {
     if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*Loc)))
       return &cast<AggregateStorageLocation>(Val->getPointeeLoc());
     return nullptr;
   }
   return cast<AggregateStorageLocation>(Loc);
 }
 
+std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD) {
+  // Unnamed bitfields are only used for padding and do not appear in
+  // `InitListExpr`'s inits. However, those fields do appear in `RecordDecl`'s
+  // field list, and we thus need to remove them before mapping inits to
+  // fields to avoid mapping inits to the wrongs fields.
+  std::vector<FieldDecl *> Fields;
+  llvm::copy_if(
+      RD->fields(), std::back_inserter(Fields),
+      [](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
+  return Fields;
+}
+
 } // namespace dataflow
 } // namespace clang

clang/lib/Analysis/FlowSensitive/Transfer.cpp

void VisitInitListExpr(const InitListExpr *S) {
 
     auto *Val = Env.createValue(Type);
     if (Val == nullptr)
       return;
 
     Env.setValue(Loc, *Val);
 
     if (Type->isStructureOrClassType()) {
-      // Unnamed bitfields are only used for padding and are not appearing in
-      // `InitListExpr`'s inits. However, those fields do appear in RecordDecl's
-      // field list, and we thus need to remove them before mapping inits to
-      // fields to avoid mapping inits to the wrongs fields.
-      std::vector<FieldDecl *> Fields;
-      llvm::copy_if(
-          Type->getAsRecordDecl()->fields(), std::back_inserter(Fields),
-          [](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
+      std::vector<FieldDecl *> Fields =
+          getFieldsForInitListExpr(Type->getAsRecordDecl());
       for (auto It : llvm::zip(Fields, S->inits())) {
         const FieldDecl *Field = std::get<0>(It);
         assert(Field != nullptr);
 
         const Expr *Init = std::get<1>(It);
         assert(Init != nullptr);
 
         if (Value *InitVal = Env.getValue(*Init, SkipPast::None))

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

std::string BracesCode = R"(
     struct B {
       int Bar;
       A Baz;
       int Qux;
     };
 
     void target(int BarArg, int FooArg, int QuxArg) {
       B Quux{BarArg, {FooArg}, QuxArg};
+      B OtherB;
       /*[[p]]*/
     }
   )";
   std::string BraceElisionCode = R"(
     struct A {
       int Foo;
     };
 
     struct B {
       int Bar;
       A Baz;
       int Qux;
     };
 
     void target(int BarArg, int FooArg, int QuxArg) {
       B Quux = {BarArg, FooArg, QuxArg};
+      B OtherB;
       /*[[p]]*/
     }
   )";
   for (const std::string &Code : {BracesCode, BraceElisionCode}) {
     runDataflow(
         Code,
         [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
            ASTContext &ASTCtx) {
runDataflow(
 
           const auto *BazVal =
               cast<StructValue>(getFieldValue(QuuxVal, *BazDecl, Env));
           ASSERT_THAT(BazVal, NotNull());
 
           EXPECT_EQ(getFieldValue(QuuxVal, *BarDecl, Env), BarArgVal);
           EXPECT_EQ(getFieldValue(BazVal, *FooDecl, Env), FooArgVal);
           EXPECT_EQ(getFieldValue(QuuxVal, *QuxDecl, Env), QuxArgVal);
+
+          // Check that fields initialized in an initializer list are always
+          // modeled in other instances of the same type.
+          const auto &OtherBVal =
+              getValueForDecl<StructValue>(ASTCtx, Env, "OtherB");
+          EXPECT_THAT(OtherBVal.getChild(*BarDecl), NotNull());
+          EXPECT_THAT(OtherBVal.getChild(*BazDecl), NotNull());
+          EXPECT_THAT(OtherBVal.getChild(*QuxDecl), NotNull());
         });
   }
 }
 
 TEST(TransferTest, AssignToUnionMember) {
   std::string Code = R"(
     union A {
       int Foo;