[clang] 2902ea3 - [clang][dataflow] Introduce `getFieldValue()` test helpers.

Martin Braenne via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 11 21:52:45 PDT 2023


Author: Martin Braenne
Date: 2023-07-12T04:52:23Z
New Revision: 2902ea3d817bf381817ff76228c3212f4dc87d47

URL: https://github.com/llvm/llvm-project/commit/2902ea3d817bf381817ff76228c3212f4dc87d47
DIFF: https://github.com/llvm/llvm-project/commit/2902ea3d817bf381817ff76228c3212f4dc87d47.diff

LOG: [clang][dataflow] Introduce `getFieldValue()` test helpers.

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

Reviewed By: ymandel, xazax.hun, gribozavr2

Differential Revision: https://reviews.llvm.org/D154935

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index 1c78dd380c7742..17c0f7b16c1c71 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/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 @@ namespace {
 
 using namespace clang;
 using namespace dataflow;
+using ::clang::dataflow::test::getFieldValue;
 using ::testing::ElementsAre;
 using ::testing::NotNull;
 using ::testing::Pair;
@@ -89,14 +91,9 @@ TEST_F(EnvironmentTest, CreateValueRecursiveType) {
   // 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 = 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 @@ TEST_F(EnvironmentTest, IncludeFieldsFromDefaultInitializers) {
   // 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 @@ TEST_F(EnvironmentTest, InitGlobalVarsFieldFun) {
   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));
 }
 

diff  --git a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
index 1f5fce1f2dd467..dc81e9594b6914 100644
--- a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -63,12 +63,12 @@ TEST(RecordOpsTest, CopyRecord) {
         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 @@ TEST(RecordOpsTest, CopyRecord) {
 
         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));

diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index 1568fc9a200e47..aa1a0f0e773a9a 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -451,6 +451,28 @@ ValueT &getValueForDecl(ASTContext &ASTCtx, const Environment &Env,
   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;


        


More information about the cfe-commits mailing list