[PATCH] D127312: [clang][dataflow] Remove IndirectionValue class, moving PointeeLoc field into PointerValue and ReferenceValue

2022-06-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
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

2022-06-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
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

2022-06-08 Thread weiyi via Phabricator via cfe-commits
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

2022-06-08 Thread weiyi via Phabricator via cfe-commits
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

2022-06-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
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