mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a project: All.
mboehme requested review of this revision.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.

These insulate tests against changes to the `getChild()` functions of
`AggregateStorageLocation` and `StructValue` that will happen as part of the
migration to strict handling of value categories (see
https://discourse.llvm.org/t/70086 for details):

- `AggregateStorageLocation::getChild()` will soon return a `StorageLocation *` 
instead of a `StorageLocation &`. When this happens, `getFieldValue()` will be 
changed to return null if `AggregateStorageLocation::getChild()` returns null; 
test code will not need to change as it should already be checking whether the 
return value of `getFieldValue()` is null.

- `StructValue::getChild()` will soon return a `StorageLocation *` instead of a 
`Value *`. When this happens, `getFieldValue()` will be changed to look up the 
`Value *` in the `Environment`. Again, test code will not need to change.

The test helpers will continue to serve a useful purpose once the API changes
are complete, so the intent is to leave them in place.

This patch changes DataflowEnvironmentTest.cpp and RecordOpsTest.cpp to use the
test helpers. TransferTest.cpp will be changed in an upcoming patch to help keep
patch sizes manageable for review.

Depends On D154934 <https://reviews.llvm.org/D154934>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154935

Files:
  clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
  clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -451,6 +451,28 @@
   return *cast<ValueT>(Env.getValue(*VD));
 }
 
+/// Returns the value of a `Field` on the record referenced by `Loc.`
+/// Returns null if `Loc` is null.
+inline Value *getFieldValue(const AggregateStorageLocation *Loc,
+                            const ValueDecl &Field, const Environment &Env) {
+  if (Loc == nullptr)
+    return nullptr;
+  return Env.getValue(Loc->getChild(Field));
+}
+
+/// Returns the value of a `Field` on a `Struct.
+/// Returns null if `Struct` is null.
+///
+/// Note: This function currently does not use the `Env` parameter, but it will
+/// soon be needed to look up the `Value` when `setChild()` changes to return a
+/// `StorageLocation *`.
+inline Value *getFieldValue(const StructValue *Struct, const ValueDecl &Field,
+                            const Environment &Env) {
+  if (Struct == nullptr)
+    return nullptr;
+  return Struct->getChild(Field);
+}
+
 /// Creates and owns constraints which are boolean values.
 class ConstraintContext {
   unsigned NextAtom = 0;
Index: clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -63,12 +63,12 @@
         auto &Inner1 = cast<AggregateStorageLocation>(S1.getChild(*InnerDecl));
         auto &Inner2 = cast<AggregateStorageLocation>(S2.getChild(*InnerDecl));
 
-        EXPECT_NE(Env.getValue(S1.getChild(*OuterIntDecl)),
-                  Env.getValue(S2.getChild(*OuterIntDecl)));
+        EXPECT_NE(getFieldValue(&S1, *OuterIntDecl, Env),
+                  getFieldValue(&S2, *OuterIntDecl, Env));
         EXPECT_NE(Env.getValue(S1.getChild(*RefDecl)),
                   Env.getValue(S2.getChild(*RefDecl)));
-        EXPECT_NE(Env.getValue(Inner1.getChild(*InnerIntDecl)),
-                  Env.getValue(Inner2.getChild(*InnerIntDecl)));
+        EXPECT_NE(getFieldValue(&Inner1, *InnerIntDecl, Env),
+                  getFieldValue(&Inner2, *InnerIntDecl, Env));
 
         auto *S1Val = cast<StructValue>(Env.getValue(S1));
         auto *S2Val = cast<StructValue>(Env.getValue(S2));
@@ -78,12 +78,12 @@
 
         copyRecord(S1, S2, Env);
 
-        EXPECT_EQ(Env.getValue(S1.getChild(*OuterIntDecl)),
-                  Env.getValue(S2.getChild(*OuterIntDecl)));
+        EXPECT_EQ(getFieldValue(&S1, *OuterIntDecl, Env),
+                  getFieldValue(&S2, *OuterIntDecl, Env));
         EXPECT_EQ(Env.getValue(S1.getChild(*RefDecl)),
                   Env.getValue(S2.getChild(*RefDecl)));
-        EXPECT_EQ(Env.getValue(Inner1.getChild(*InnerIntDecl)),
-                  Env.getValue(Inner2.getChild(*InnerIntDecl)));
+        EXPECT_EQ(getFieldValue(&Inner1, *InnerIntDecl, Env),
+                  getFieldValue(&Inner2, *InnerIntDecl, Env));
 
         S1Val = cast<StructValue>(Env.getValue(S1));
         S2Val = cast<StructValue>(Env.getValue(S2));
Index: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "TestingSupport.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
@@ -23,6 +24,7 @@
 
 using namespace clang;
 using namespace dataflow;
+using test::getFieldValue;
 using ::testing::ElementsAre;
 using ::testing::NotNull;
 using ::testing::Pair;
@@ -89,14 +91,9 @@
   // Verify that the struct and the field (`R`) with first appearance of the
   // type is created successfully.
   Environment Env(DAContext, *Fun);
-  Value *Val = Env.createValue(Ty);
-  ASSERT_NE(Val, nullptr);
-  StructValue *SVal = clang::dyn_cast<StructValue>(Val);
-  ASSERT_NE(SVal, nullptr);
-  Val = SVal->getChild(*R);
-  ASSERT_NE(Val, nullptr);
-  PointerValue *PV = clang::dyn_cast<PointerValue>(Val);
-  EXPECT_NE(PV, nullptr);
+  StructValue *SVal = clang::cast<StructValue>(Env.createValue(Ty));
+  PointerValue *PV = cast_or_null<PointerValue>(getFieldValue(SVal, *R, Env));
+  EXPECT_THAT(PV, NotNull());
 }
 
 TEST_F(EnvironmentTest, InitGlobalVarsFun) {
@@ -175,8 +172,7 @@
   // constructor, even though it is not referenced directly in the constructor.
   Environment Env(DAContext, *Constructor);
   auto *Val = cast<StructValue>(Env.createValue(QTy));
-  ASSERT_THAT(Val, NotNull());
-  EXPECT_THAT(Val->getChild(*XDecl), NotNull());
+  EXPECT_THAT(getFieldValue(Val, *XDecl, Env), NotNull());
 }
 
 TEST_F(EnvironmentTest, InitGlobalVarsFieldFun) {
@@ -221,8 +217,7 @@
   const auto *GlobalLoc =
       cast<AggregateStorageLocation>(Env.getStorageLocation(*GlobalDecl));
   const auto *GlobalVal = cast<StructValue>(Env.getValue(*GlobalLoc));
-  const auto *BarVal = GlobalVal->getChild(*BarDecl);
-  ASSERT_THAT(BarVal, NotNull());
+  auto *BarVal = getFieldValue(GlobalVal, *BarDecl, Env);
   EXPECT_TRUE(isa<IntegerValue>(BarVal));
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to