ymandel updated this revision to Diff 423724.
ymandel added a comment.
extended comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123961/new/
https://reviews.llvm.org/D123961
Files:
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -187,6 +187,54 @@
});
}
+TEST_F(TransferTest, StructVarDeclWithInit) {
+ std::string Code = R"(
+ struct A {
+ int Bar;
+ };
+
+ A Gen();
+
+ void target() {
+ A Foo = Gen();
+ // [[p]]
+ }
+ )";
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ const Environment &Env = Results[0].second.Env;
+
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+
+ ASSERT_TRUE(FooDecl->getType()->isStructureType());
+ auto FooFields = FooDecl->getType()->getAsRecordDecl()->fields();
+
+ FieldDecl *BarDecl = nullptr;
+ for (FieldDecl *Field : FooFields) {
+ if (Field->getNameAsString() == "Bar") {
+ BarDecl = Field;
+ } else {
+ FAIL() << "Unexpected field: " << Field->getNameAsString();
+ }
+ }
+ ASSERT_THAT(BarDecl, NotNull());
+
+ const auto *FooLoc = cast<AggregateStorageLocation>(
+ Env.getStorageLocation(*FooDecl, SkipPast::None));
+ const auto *BarLoc =
+ cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
+
+ const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
+ const auto *BarVal = cast<IntegerValue>(FooVal->getChild(*BarDecl));
+ EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
+ });
+}
+
TEST_F(TransferTest, ClassVarDecl) {
std::string Code = R"(
class A {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -168,27 +168,25 @@
auto &Val =
Env.takeOwnership(std::make_unique<ReferenceValue>(*InitExprLoc));
Env.setValue(Loc, Val);
- } else {
- // FIXME: The initializer expression must always be assigned a value.
- // Replace this with an assert when we have sufficient coverage of
- // language features.
- if (Value *Val = Env.createValue(D.getType()))
- Env.setValue(Loc, *Val);
+ return;
}
+ } else if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
+ Env.setValue(Loc, *InitExprVal);
return;
}
- if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
- Env.setValue(Loc, *InitExprVal);
- } else if (!D.getType()->isStructureOrClassType()) {
- // FIXME: The initializer expression must always be assigned a value.
- // Replace this with an assert when we have sufficient coverage of
- // language features.
- if (Value *Val = Env.createValue(D.getType()))
- Env.setValue(Loc, *Val);
- } else {
- llvm_unreachable("structs and classes must always be assigned values");
- }
+ // We arrive here in (the few) cases where an expression is intentionally
+ // "uninterpreted". There are two ways to handle this situation: propagate
+ // the status, so that uninterpreted initializers result in uninterpreted
+ // variables, or provide a default value. We choose the latter so that later
+ // refinements of the variable can be used for reasoning about the
+ // surrounding code.
+ //
+ // FIXME. If and when we interpret all language cases, change this to assert
+ // that `InitExpr` is interpreted, rather than supplying a default value
+ // (assuming we don't update the environment API to return references).
+ if (Value *Val = Env.createValue(D.getType()))
+ Env.setValue(Loc, *Val);
}
void VisitImplicitCastExpr(const ImplicitCastExpr *S) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
