[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue
This revision was automatically updated to reflect the committed changes. Closed by commit rGa1b2b7d9790b: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into… (authored by wyt, committed by gribozavr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127312/new/ https://reviews.llvm.org/D127312 Files: clang/include/clang/Analysis/FlowSensitive/Value.h clang/lib/Analysis/FlowSensitive/DataflowEnvironment.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 @@ -122,24 +122,24 @@ // [[p]] } )"; - runDataflow( - Code, [](llvm::ArrayRef< - std::pair>> - Results, - ASTContext ) { -ASSERT_THAT(Results, ElementsAre(Pair("p", _))); -const Environment = Results[0].second.Env; + runDataflow(Code, + [](llvm::ArrayRef< + std::pair>> + Results, + ASTContext ) { +ASSERT_THAT(Results, ElementsAre(Pair("p", _))); +const Environment = Results[0].second.Env; -const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); -ASSERT_THAT(FooDecl, NotNull()); +const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); +ASSERT_THAT(FooDecl, NotNull()); -const StorageLocation *FooLoc = -Env.getStorageLocation(*FooDecl, SkipPast::None); -ASSERT_TRUE(isa_and_nonnull(FooLoc)); +const StorageLocation *FooLoc = +Env.getStorageLocation(*FooDecl, SkipPast::None); +ASSERT_TRUE(isa_and_nonnull(FooLoc)); -const Value *FooVal = Env.getValue(*FooLoc); -EXPECT_TRUE(isa_and_nonnull(FooVal)); - }); +const Value *FooVal = Env.getValue(*FooLoc); +EXPECT_TRUE(isa_and_nonnull(FooVal)); + }); } TEST_F(TransferTest, StructVarDecl) { @@ -293,29 +293,29 @@ // [[p]] } )"; - runDataflow( - Code, [](llvm::ArrayRef< - std::pair>> - Results, - ASTContext ) { -ASSERT_THAT(Results, ElementsAre(Pair("p", _))); -const Environment = Results[0].second.Env; + runDataflow(Code, + [](llvm::ArrayRef< + std::pair>> + Results, + ASTContext ) { +ASSERT_THAT(Results, ElementsAre(Pair("p", _))); +const Environment = Results[0].second.Env; -const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); -ASSERT_THAT(FooDecl, NotNull()); +const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); +ASSERT_THAT(FooDecl, NotNull()); -const StorageLocation *FooLoc = -Env.getStorageLocation(*FooDecl, SkipPast::None); -ASSERT_TRUE(isa_and_nonnull(FooLoc)); +const StorageLocation *FooLoc = +Env.getStorageLocation(*FooDecl, SkipPast::None); +ASSERT_TRUE(isa_and_nonnull(FooLoc)); -const ReferenceValue *FooVal = -cast(Env.getValue(*FooLoc)); -const StorageLocation = FooVal->getPointeeLoc(); -EXPECT_TRUE(isa()); +const ReferenceValue *FooVal = +cast(Env.getValue(*FooLoc)); +const StorageLocation = FooVal->getPointeeLoc(); +EXPECT_TRUE(isa()); -const Value *FooPointeeVal = Env.getValue(FooPointeeLoc); -EXPECT_TRUE(isa_and_nonnull(FooPointeeVal)); - }); +const Value *FooPointeeVal = Env.getValue(FooPointeeLoc); +EXPECT_TRUE(isa_and_nonnull(FooPointeeVal)); + }); } TEST_F(TransferTest, SelfReferentialReferenceVarDecl) { @@ -3327,23 +3327,23 @@ ASSERT_THAT(LDecl, NotNull()); // Inner. -auto *LVal = dyn_cast( -InnerEnv.getValue(*LDecl, SkipPast::None)); +auto *LVal = +dyn_cast(InnerEnv.getValue(*LDecl, SkipPast::None)); ASSERT_THAT(LVal, NotNull()); EXPECT_EQ(>getPointeeLoc(), InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); // Outer. -LVal = dyn_cast( -OuterEnv.getValue(*LDecl, SkipPast::None)); +LVal = +dyn_cast(OuterEnv.getValue(*LDecl, SkipPast::None)); ASSERT_THAT(LVal, NotNull()); // The loop body may not have been executed, so we should not conclude // that `l` points to `val`. EXPECT_NE(>getPointeeLoc(), OuterEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); -}); +
[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:63 -auto *IndVal2 = cast(Val2); -assert(IndVal1->getKind() == IndVal2->getKind()); -if (>getPointeeLoc() == >getPointeeLoc()) wyt wrote: > gribozavr2 wrote: > > This assert was lost in the new implementation. > The assertion was to ensure that the values were either both Reference or > both Pointer, as an IndirectionValue could be either. > Since we've removed IndirectionVal and have separate casts for > ReferenceVal/PointerVal, the assertion is not neccessary. Ah I see, the `cast` now plays the role of the assert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127312/new/ https://reviews.llvm.org/D127312 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue
wyt added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:63 -auto *IndVal2 = cast(Val2); -assert(IndVal1->getKind() == IndVal2->getKind()); -if (>getPointeeLoc() == >getPointeeLoc()) gribozavr2 wrote: > This assert was lost in the new implementation. The assertion was to ensure that the values were either both Reference or both Pointer, as an IndirectionValue could be either. Since we've removed IndirectionVal and have separate casts for ReferenceVal/PointerVal, the assertion is not neccessary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127312/new/ https://reviews.llvm.org/D127312 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue
wyt updated this revision to Diff 435271. wyt marked 2 inline comments as done. wyt added a comment. Rename function as suggested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127312/new/ https://reviews.llvm.org/D127312 Files: clang/include/clang/Analysis/FlowSensitive/Value.h clang/lib/Analysis/FlowSensitive/DataflowEnvironment.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 @@ -122,24 +122,24 @@ // [[p]] } )"; - runDataflow( - Code, [](llvm::ArrayRef< - std::pair>> - Results, - ASTContext ) { -ASSERT_THAT(Results, ElementsAre(Pair("p", _))); -const Environment = Results[0].second.Env; + runDataflow(Code, + [](llvm::ArrayRef< + std::pair>> + Results, + ASTContext ) { +ASSERT_THAT(Results, ElementsAre(Pair("p", _))); +const Environment = Results[0].second.Env; -const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); -ASSERT_THAT(FooDecl, NotNull()); +const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); +ASSERT_THAT(FooDecl, NotNull()); -const StorageLocation *FooLoc = -Env.getStorageLocation(*FooDecl, SkipPast::None); -ASSERT_TRUE(isa_and_nonnull(FooLoc)); +const StorageLocation *FooLoc = +Env.getStorageLocation(*FooDecl, SkipPast::None); +ASSERT_TRUE(isa_and_nonnull(FooLoc)); -const Value *FooVal = Env.getValue(*FooLoc); -EXPECT_TRUE(isa_and_nonnull(FooVal)); - }); +const Value *FooVal = Env.getValue(*FooLoc); +EXPECT_TRUE(isa_and_nonnull(FooVal)); + }); } TEST_F(TransferTest, StructVarDecl) { @@ -293,29 +293,29 @@ // [[p]] } )"; - runDataflow( - Code, [](llvm::ArrayRef< - std::pair>> - Results, - ASTContext ) { -ASSERT_THAT(Results, ElementsAre(Pair("p", _))); -const Environment = Results[0].second.Env; + runDataflow(Code, + [](llvm::ArrayRef< + std::pair>> + Results, + ASTContext ) { +ASSERT_THAT(Results, ElementsAre(Pair("p", _))); +const Environment = Results[0].second.Env; -const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); -ASSERT_THAT(FooDecl, NotNull()); +const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); +ASSERT_THAT(FooDecl, NotNull()); -const StorageLocation *FooLoc = -Env.getStorageLocation(*FooDecl, SkipPast::None); -ASSERT_TRUE(isa_and_nonnull(FooLoc)); +const StorageLocation *FooLoc = +Env.getStorageLocation(*FooDecl, SkipPast::None); +ASSERT_TRUE(isa_and_nonnull(FooLoc)); -const ReferenceValue *FooVal = -cast(Env.getValue(*FooLoc)); -const StorageLocation = FooVal->getPointeeLoc(); -EXPECT_TRUE(isa()); +const ReferenceValue *FooVal = +cast(Env.getValue(*FooLoc)); +const StorageLocation = FooVal->getPointeeLoc(); +EXPECT_TRUE(isa()); -const Value *FooPointeeVal = Env.getValue(FooPointeeLoc); -EXPECT_TRUE(isa_and_nonnull(FooPointeeVal)); - }); +const Value *FooPointeeVal = Env.getValue(FooPointeeLoc); +EXPECT_TRUE(isa_and_nonnull(FooPointeeVal)); + }); } TEST_F(TransferTest, SelfReferentialReferenceVarDecl) { @@ -3327,23 +3327,23 @@ ASSERT_THAT(LDecl, NotNull()); // Inner. -auto *LVal = dyn_cast( -InnerEnv.getValue(*LDecl, SkipPast::None)); +auto *LVal = +dyn_cast(InnerEnv.getValue(*LDecl, SkipPast::None)); ASSERT_THAT(LVal, NotNull()); EXPECT_EQ(>getPointeeLoc(), InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); // Outer. -LVal = dyn_cast( -OuterEnv.getValue(*LDecl, SkipPast::None)); +LVal = +dyn_cast(OuterEnv.getValue(*LDecl, SkipPast::None)); ASSERT_THAT(LVal, NotNull()); // The loop body may not have been executed, so we should not conclude // that `l` points to `val`. EXPECT_NE(>getPointeeLoc(), OuterEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); -}); + }); } TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) { Index:
[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue
gribozavr2 added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:53 +static bool indirectionEquivalentValues(Value *Val1, Value *Val2) { + if (auto *IndVal1 = dyn_cast(Val1)) { WDYT about "areEquivalentIndirectionValues"? Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:63 -auto *IndVal2 = cast(Val2); -assert(IndVal1->getKind() == IndVal2->getKind()); -if (>getPointeeLoc() == >getPointeeLoc()) This assert was lost in the new implementation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127312/new/ https://reviews.llvm.org/D127312 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits