ymandel created this revision. ymandel added reviewers: xazax.hun, sgatev. Herald added subscribers: tschuett, steakhal, rnkovacs. Herald added a project: All. ymandel requested review of this revision. Herald added a project: clang.
Remove constraint that an initializing expression of struct type must have an associated `Value`. This invariant is not and will not be guaranteed by the framework, because of potentially uninitialized fields. Repository: rG LLVM Github Monorepo 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 @@ -178,17 +178,10 @@ return; } - if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) { + 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"); - } + else if (Value *Val = Env.createValue(D.getType())) + Env.setValue(Loc, *Val); } void VisitImplicitCastExpr(const ImplicitCastExpr *S) {
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 @@ -178,17 +178,10 @@ return; } - if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) { + 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"); - } + else 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