Skip to content

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

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 2 commits into from
Apr 19, 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
4 changes: 4 additions & 0 deletions clang/include/clang/Analysis/FlowSensitive/ASTOps.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;

Expand Down
35 changes: 25 additions & 10 deletions clang/lib/Analysis/FlowSensitive/ASTOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}

Expand Down
48 changes: 29 additions & 19 deletions clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,28 @@ 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())
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).
Expand Down Expand Up @@ -440,26 +462,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;
}

Expand Down
73 changes: 73 additions & 0 deletions clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down