Skip to content

[clang][dataflow] Introduce a helper class for handling record initializer lists. #86675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,35 @@ RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
std::vector<const FieldDecl *>
getFieldsForInitListExpr(const InitListExpr *InitList);

/// Helper class for initialization of a record with an `InitListExpr`.
/// `InitListExpr::inits()` contains the initializers for both the base classes
/// and the fields of the record; this helper class separates these out into two
/// different lists. In addition, it deals with special cases associated with
/// unions.
class RecordInitListHelper {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why an object vs something like:

struct RecordInits {
  SmallVector<std::pair<const CXXBaseSpecifier *, Expr *>> BaseInits;
  SmallVector<std::pair<const FieldDecl *, Expr *>> FieldInits;
};
RecordInits RecordInitListHelper(const InitListExpr *InitList);

Is it because of the optional storage needed for the union?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it because of the optional storage needed for the union?

Yes, exactly this -- otherwise, there would be no way for us to keep the ImplicitValueInitExpr "alive".

public:
// `InitList` must have record type.
RecordInitListHelper(const InitListExpr *InitList);

// Base classes with their associated initializer expressions.
ArrayRef<std::pair<const CXXBaseSpecifier *, Expr *>> base_inits() const {
return BaseInits;
}

// Fields with their associated initializer expressions.
ArrayRef<std::pair<const FieldDecl *, Expr *>> field_inits() const {
return FieldInits;
}

private:
SmallVector<std::pair<const CXXBaseSpecifier *, Expr *>> BaseInits;
SmallVector<std::pair<const FieldDecl *, Expr *>> FieldInits;

// We potentially synthesize an `ImplicitValueInitExpr` for unions. It's a
// member variable because we store a pointer to it in `FieldInits`.
std::optional<ImplicitValueInitExpr> ImplicitValueInitForUnion;
};

/// Associates a new `RecordValue` with `Loc` and returns the new value.
RecordValue &refreshRecordValue(RecordStorageLocation &Loc, Environment &Env);

Expand Down
36 changes: 36 additions & 0 deletions clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,42 @@ getFieldsForInitListExpr(const InitListExpr *InitList) {
return Fields;
}

RecordInitListHelper::RecordInitListHelper(const InitListExpr *InitList) {
auto *RD = InitList->getType()->getAsCXXRecordDecl();
assert(RD != nullptr);

std::vector<const FieldDecl *> Fields = getFieldsForInitListExpr(InitList);
ArrayRef<Expr *> Inits = InitList->inits();

// 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()) {
assert(Fields.size() == 1);
ImplicitValueInitForUnion.emplace(Fields.front()->getType());
InitsForUnion.push_back(&*ImplicitValueInitForUnion);
Inits = InitsForUnion;
}

size_t InitIdx = 0;

assert(Fields.size() + RD->getNumBases() == Inits.size());
for (const CXXBaseSpecifier &Base : RD->bases()) {
assert(InitIdx < Inits.size());
Expr *Init = Inits[InitIdx++];
BaseInits.emplace_back(&Base, Init);
}

assert(Fields.size() == Inits.size() - InitIdx);
for (const FieldDecl *Field : Fields) {
assert(InitIdx < Inits.size());
Expr *Init = Inits[InitIdx++];
FieldInits.emplace_back(Field, Init);
}
}

RecordValue &refreshRecordValue(RecordStorageLocation &Loc, Environment &Env) {
auto &NewVal = Env.create<RecordValue>(Loc);
Env.setValue(Loc, NewVal);
Expand Down
61 changes: 16 additions & 45 deletions clang/lib/Analysis/FlowSensitive/Transfer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -689,51 +689,22 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
}

llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;

// This only contains the direct fields for the given type.
std::vector<const FieldDecl *> FieldsForInit = getFieldsForInitListExpr(S);

// `S->inits()` contains all the initializer expressions, including the
// ones for direct base classes.
ArrayRef<Expr *> Inits = S->inits();
size_t InitIdx = 0;

// 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.
std::optional<ImplicitValueInitExpr> ImplicitValueInitForUnion;
SmallVector<Expr *> InitsForUnion;
if (S->getType()->isUnionType() && Inits.empty()) {
assert(FieldsForInit.size() == 1);
ImplicitValueInitForUnion.emplace(FieldsForInit.front()->getType());
InitsForUnion.push_back(&*ImplicitValueInitForUnion);
Inits = InitsForUnion;
}

// Initialize base classes.
if (auto* R = S->getType()->getAsCXXRecordDecl()) {
assert(FieldsForInit.size() + R->getNumBases() == Inits.size());
for ([[maybe_unused]] const CXXBaseSpecifier &Base : R->bases()) {
assert(InitIdx < Inits.size());
auto Init = Inits[InitIdx++];
assert(Base.getType().getCanonicalType() ==
Init->getType().getCanonicalType());
auto *BaseVal = Env.get<RecordValue>(*Init);
if (!BaseVal)
BaseVal = cast<RecordValue>(Env.createValue(Init->getType()));
// Take ownership of the fields of the `RecordValue` for the base class
// and incorporate them into the "flattened" set of fields for the
// derived class.
auto Children = BaseVal->getLoc().children();
FieldLocs.insert(Children.begin(), Children.end());
}
}

assert(FieldsForInit.size() == Inits.size() - InitIdx);
for (auto Field : FieldsForInit) {
assert(InitIdx < Inits.size());
auto Init = Inits[InitIdx++];
RecordInitListHelper InitListHelper(S);

for (auto [Base, Init] : InitListHelper.base_inits()) {
assert(Base->getType().getCanonicalType() ==
Init->getType().getCanonicalType());
auto *BaseVal = Env.get<RecordValue>(*Init);
if (!BaseVal)
BaseVal = cast<RecordValue>(Env.createValue(Init->getType()));
// Take ownership of the fields of the `RecordValue` for the base class
// and incorporate them into the "flattened" set of fields for the
// derived class.
auto Children = BaseVal->getLoc().children();
FieldLocs.insert(Children.begin(), Children.end());
}

for (auto [Field, Init] : InitListHelper.field_inits()) {
assert(
// The types are same, or
Field->getType().getCanonicalType().getUnqualifiedType() ==
Expand Down