Skip to content

Commit b47bdcb

Browse files
committed
[clang][dataflow] Include fields initialized in an InitListExpr in getModeledFields().
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 Reviewed By: xazax.hun, gribozavr2 Differential Revision: https://reviews.llvm.org/D154961
1 parent 1e9b4fc commit b47bdcb

File tree

4 files changed

+32
-8
lines changed

4 files changed

+32
-8
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,10 @@ getImplicitObjectLocation(const CXXMemberCallExpr &MCE, const Environment &Env);
644644
AggregateStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
645645
const Environment &Env);
646646

647+
/// Returns the fields of `RD` that are initialized by an `InitListExpr`, in the
648+
/// order in which they appear in `InitListExpr::inits()`.
649+
std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD);
650+
647651
} // namespace dataflow
648652
} // namespace clang
649653

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,10 @@ getFieldsGlobalsAndFuncs(const Stmt &S, FieldSet &Fields,
215215
insertIfFunction(*VD, Funcs);
216216
if (const auto *FD = dyn_cast<FieldDecl>(VD))
217217
Fields.insert(FD);
218+
} else if (auto *InitList = dyn_cast<InitListExpr>(&S)) {
219+
if (RecordDecl *RD = InitList->getType()->getAsRecordDecl())
220+
for (const auto *FD : getFieldsForInitListExpr(RD))
221+
Fields.insert(FD);
218222
}
219223
}
220224

@@ -958,5 +962,17 @@ AggregateStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
958962
return cast<AggregateStorageLocation>(Loc);
959963
}
960964

965+
std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD) {
966+
// Unnamed bitfields are only used for padding and do not appear in
967+
// `InitListExpr`'s inits. However, those fields do appear in `RecordDecl`'s
968+
// field list, and we thus need to remove them before mapping inits to
969+
// fields to avoid mapping inits to the wrongs fields.
970+
std::vector<FieldDecl *> Fields;
971+
llvm::copy_if(
972+
RD->fields(), std::back_inserter(Fields),
973+
[](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
974+
return Fields;
975+
}
976+
961977
} // namespace dataflow
962978
} // namespace clang

clang/lib/Analysis/FlowSensitive/Transfer.cpp

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -715,14 +715,8 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
715715
Env.setValue(Loc, *Val);
716716

717717
if (Type->isStructureOrClassType()) {
718-
// Unnamed bitfields are only used for padding and are not appearing in
719-
// `InitListExpr`'s inits. However, those fields do appear in RecordDecl's
720-
// field list, and we thus need to remove them before mapping inits to
721-
// fields to avoid mapping inits to the wrongs fields.
722-
std::vector<FieldDecl *> Fields;
723-
llvm::copy_if(
724-
Type->getAsRecordDecl()->fields(), std::back_inserter(Fields),
725-
[](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
718+
std::vector<FieldDecl *> Fields =
719+
getFieldsForInitListExpr(Type->getAsRecordDecl());
726720
for (auto It : llvm::zip(Fields, S->inits())) {
727721
const FieldDecl *Field = std::get<0>(It);
728722
assert(Field != nullptr);

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2833,6 +2833,7 @@ TEST(TransferTest, AggregateInitialization) {
28332833
28342834
void target(int BarArg, int FooArg, int QuxArg) {
28352835
B Quux{BarArg, {FooArg}, QuxArg};
2836+
B OtherB;
28362837
/*[[p]]*/
28372838
}
28382839
)";
@@ -2849,6 +2850,7 @@ TEST(TransferTest, AggregateInitialization) {
28492850
28502851
void target(int BarArg, int FooArg, int QuxArg) {
28512852
B Quux = {BarArg, FooArg, QuxArg};
2853+
B OtherB;
28522854
/*[[p]]*/
28532855
}
28542856
)";
@@ -2898,6 +2900,14 @@ TEST(TransferTest, AggregateInitialization) {
28982900
EXPECT_EQ(getFieldValue(QuuxVal, *BarDecl, Env), BarArgVal);
28992901
EXPECT_EQ(getFieldValue(BazVal, *FooDecl, Env), FooArgVal);
29002902
EXPECT_EQ(getFieldValue(QuuxVal, *QuxDecl, Env), QuxArgVal);
2903+
2904+
// Check that fields initialized in an initializer list are always
2905+
// modeled in other instances of the same type.
2906+
const auto &OtherBVal =
2907+
getValueForDecl<StructValue>(ASTCtx, Env, "OtherB");
2908+
EXPECT_THAT(OtherBVal.getChild(*BarDecl), NotNull());
2909+
EXPECT_THAT(OtherBVal.getChild(*BazDecl), NotNull());
2910+
EXPECT_THAT(OtherBVal.getChild(*QuxDecl), NotNull());
29012911
});
29022912
}
29032913
}

0 commit comments

Comments
 (0)