[clang] d17f455 - [clang][dataflow] Add `refreshStructValue()`.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-07-17T07:26:13Z
New Revision: d17f455a6348806c73641e742af08b0a974e13d5

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

LOG: [clang][dataflow] Add `refreshStructValue()`.

Besides being a useful abstraction, this function will help insulate existing clients of the framework from upcoming changes to the API of `StructValue` and `AggregateStorageLocation`.

Depends On D155202

Reviewed By: ymandel, xazax.hun

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

Added: 
    

Modified: 
    clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
    clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
    clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index dc82ccea4a27d4..9d99b6771f71e1 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -389,6 +389,12 @@ class Environment {
   /// necessary storage locations and values for indirections until it finds a
   /// non-pointer/non-reference type.
   ///
+  /// If `Type` is one of the following types, this function will always return
+  /// a non-null pointer:
+  /// - `bool`
+  /// - Any integer type
+  /// - Any class, struct, or union type
+  ///
   /// Requirements:
   ///
   ///  `Type` must not be null.
@@ -692,6 +698,24 @@ AggregateStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
 /// order in which they appear in `InitListExpr::inits()`.
 std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD);
 
+/// Associates a new `StructValue` with `Loc` and returns the new value.
+/// It is not defined whether the field values remain the same or not.
+///
+/// This function is primarily intended for use by checks that set custom
+/// properties on `StructValue`s to model the state of these values. Such checks
+/// should avoid modifying the properties of an existing `StructValue` because
+/// these changes would be visible to other `Environment`s that share the same
+/// `StructValue`. Instead, call `refreshStructValue()`, then set the properties
+/// on the new `StructValue` that it returns. Typical usage:
+///
+///   refreshStructValue(Loc, Env).setProperty("my_prop", MyPropValue);
+StructValue &refreshStructValue(AggregateStorageLocation &Loc,
+                                Environment &Env);
+
+/// Associates a new `StructValue` with `Expr` and returns the new value.
+/// See also documentation for the overload above.
+StructValue &refreshStructValue(const Expr &Expr, Environment &Env);
+
 } // namespace dataflow
 } // namespace clang
 

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index fdd6b5959cd722..5d301b815d38e0 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -987,5 +987,30 @@ std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD) {
   return Fields;
 }
 
+StructValue &refreshStructValue(AggregateStorageLocation &Loc,
+                                Environment &Env) {
+  auto &NewVal = *cast<StructValue>(Env.createValue(Loc.getType()));
+  Env.setValue(Loc, NewVal);
+  return NewVal;
+}
+
+StructValue &refreshStructValue(const Expr &Expr, Environment &Env) {
+  assert(Expr.getType()->isRecordType());
+
+  auto &NewVal = *cast<StructValue>(Env.createValue(Expr.getType()));
+
+  if (Expr.isPRValue()) {
+    Env.setValueStrict(Expr, NewVal);
+  } else {
+    StorageLocation *Loc = Env.getStorageLocationStrict(Expr);
+    if (Loc == nullptr) {
+      Loc = &Env.createStorageLocation(Expr);
+    }
+    Env.setValue(*Loc, NewVal);
+  }
+
+  return NewVal;
+}
+
 } // namespace dataflow
 } // namespace clang

diff  --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 11b346fbf64f3b..462ffb5eb389ce 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -370,13 +370,6 @@ TEST_F(NoreturnDestructorTest, ConditionalOperatorNestedBranchReturns) {
   // FIXME: Called functions at point `p` should contain only "foo".
 }
 
-StructValue &createNewStructValue(AggregateStorageLocation &Loc,
-                                  Environment &Env) {
-  auto &Val = *cast<StructValue>(Env.createValue(Loc.getType()));
-  Env.setValue(Loc, Val);
-  return Val;
-}
-
 // Models an analysis that uses flow conditions.
 class SpecialBoolAnalysis final
     : public DataflowAnalysis<SpecialBoolAnalysis, NoopLattice> {
@@ -407,7 +400,7 @@ class SpecialBoolAnalysis final
       auto &ObjectLoc =
           *cast<AggregateStorageLocation>(getImplicitObjectLocation(*E, Env));
 
-      createNewStructValue(ObjectLoc, Env)
+      refreshStructValue(ObjectLoc, Env)
           .setProperty("is_set", Env.getBoolLiteralValue(true));
     }
   }
@@ -562,10 +555,7 @@ class OptionalIntAnalysis final
       auto *Object = E->getArg(0);
       assert(Object != nullptr);
 
-      auto &ObjectLoc = *cast<AggregateStorageLocation>(
-          Env.getStorageLocation(*Object, SkipPast::Reference));
-
-      createNewStructValue(ObjectLoc, Env)
+      refreshStructValue(*Object, Env)
           .setProperty("has_value", Env.getBoolLiteralValue(true));
     }
   }