[clang] 17ba278 - [clang][dataflow] Remove deprecated accessors as well as `SkipPast`.

Martin Braenne via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 31 12:40:19 PDT 2023


Author: Martin Braenne
Date: 2023-07-31T19:40:06Z
New Revision: 17ba278f76116b12eb7a954c8963ee5a5a19c32b

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

LOG: [clang][dataflow] Remove deprecated accessors as well as `SkipPast`.

Depends On D156672

Reviewed By: ymandel, xazax.hun

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 460727973ed4c8..bd89c7e5c74a3c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -38,19 +38,6 @@
 namespace clang {
 namespace dataflow {
 
-/// Indicates what kind of indirections should be skipped past when retrieving
-/// storage locations or values.
-///
-/// FIXME: Consider renaming this or replacing it with a more appropriate model.
-/// See the discussion in https://reviews.llvm.org/D116596 for context.
-enum class SkipPast {
-  /// No indirections should be skipped past.
-  None,
-  /// An optional reference should be skipped past.
-  /// This is deprecated; it is equivalent to `None` and will be removed.
-  Reference,
-};
-
 /// Indicates the result of a tentative comparison.
 enum class ComparisonResult {
   Same,
@@ -280,40 +267,15 @@ class Environment {
   /// refers directly to the referenced object, not a `ReferenceValue`.
   StorageLocation *getStorageLocation(const ValueDecl &D) const;
 
-  /// Assigns `Loc` as the storage location of `E` in the environment.
-  ///
-  /// This function is deprecated; prefer `setStorageLocationStrict()`.
-  /// For details, see https://discourse.llvm.org/t/70086.
-  ///
-  /// Requirements:
-  ///
-  ///  `E` must not be assigned a storage location in the environment.
-  void setStorageLocation(const Expr &E, StorageLocation &Loc);
-
   /// Assigns `Loc` as the storage location of the glvalue `E` in the
   /// environment.
   ///
-  /// This function is the preferred alternative to
-  /// `setStorageLocation(const Expr &, StorageLocation &)`. Once the migration
-  /// to strict handling of value categories is complete (see
-  /// https://discourse.llvm.org/t/70086), `setStorageLocation()` will be
-  /// removed and this function will be renamed to `setStorageLocation()`.
-  ///
   /// Requirements:
   ///
   ///  `E` must not be assigned a storage location in the environment.
   ///  `E` must be a glvalue or a `BuiltinType::BuiltinFn`
   void setStorageLocationStrict(const Expr &E, StorageLocation &Loc);
 
-  /// Returns the storage location assigned to `E` in the environment, or null
-  /// if `E` isn't assigned a storage location in the environment.
-  ///
-  /// The `SP` parameter has no effect.
-  ///
-  /// This function is deprecated; prefer `getStorageLocationStrict()`.
-  /// For details, see https://discourse.llvm.org/t/70086.
-  StorageLocation *getStorageLocation(const Expr &E, SkipPast SP) const;
-
   /// Returns the storage location assigned to the glvalue `E` in the
   /// environment, or null if `E` isn't assigned a storage location in the
   /// environment.
@@ -321,12 +283,6 @@ class Environment {
   /// If the storage location for `E` is associated with a
   /// `ReferenceValue RefVal`, returns `RefVal.getReferentLoc()` instead.
   ///
-  /// This function is the preferred alternative to
-  /// `getStorageLocation(const Expr &, SkipPast)`. Once the migration
-  /// to strict handling of value categories is complete (see
-  /// https://discourse.llvm.org/t/70086), `getStorageLocation()` will be
-  /// removed and this function will be renamed to `getStorageLocation()`.
-  ///
   /// Requirements:
   ///  `E` must be a glvalue or a `BuiltinType::BuiltinFn`
   StorageLocation *getStorageLocationStrict(const Expr &E) const;
@@ -498,20 +454,7 @@ class Environment {
 
   /// Equivalent to `getValue(getStorageLocation(E, SP))` if `E` is assigned a
   /// storage location in the environment, otherwise returns null.
-  ///
-  /// The `SP` parameter is deprecated and has no effect. New callers should
-  /// avoid passing this parameter.
-  Value *getValue(const Expr &E, SkipPast SP = SkipPast::None) const;
-
-  /// Returns the `Value` assigned to the prvalue `E` in the environment, or
-  /// null if `E` isn't assigned a value in the environment.
-  ///
-  /// This function is deprecated. Call `getValue(E)` instead.
-  ///
-  /// Requirements:
-  ///
-  ///  `E` must be a prvalue
-  Value *getValueStrict(const Expr &E) const;
+  Value *getValue(const Expr &E) const;
 
   // FIXME: should we deprecate the following & call arena().create() directly?
 
@@ -642,6 +585,14 @@ class Environment {
   // The copy-constructor is for use in fork() only.
   Environment(const Environment &) = default;
 
+  /// Internal version of `setStorageLocationStrict()` that doesn't check if the
+  /// expression is a prvalue.
+  void setStorageLocationInternal(const Expr &E, StorageLocation &Loc);
+
+  /// Internal version of `getStorageLocationStrict()` that doesn't check if the
+  /// expression is a prvalue.
+  StorageLocation *getStorageLocationInternal(const Expr &E) const;
+
   /// Creates a value appropriate for `Type`, if `Type` is supported, otherwise
   /// return null.
   ///

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index c1cd00a73491ae..48f8e63a165efe 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -618,12 +618,6 @@ StorageLocation *Environment::getStorageLocation(const ValueDecl &D) const {
   return Loc;
 }
 
-void Environment::setStorageLocation(const Expr &E, StorageLocation &Loc) {
-  const Expr &CanonE = ignoreCFGOmittedNodes(E);
-  assert(!ExprToLoc.contains(&CanonE));
-  ExprToLoc[&CanonE] = &Loc;
-}
-
 void Environment::setStorageLocationStrict(const Expr &E,
                                            StorageLocation &Loc) {
   // `DeclRefExpr`s to builtin function types aren't glvalues, for some reason,
@@ -631,26 +625,14 @@ void Environment::setStorageLocationStrict(const Expr &E,
   // so allow these as an exception.
   assert(E.isGLValue() ||
          E.getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn));
-  setStorageLocation(E, Loc);
-}
-
-StorageLocation *Environment::getStorageLocation(const Expr &E,
-                                                 SkipPast SP) const {
-  // FIXME: Add a test with parens.
-  auto It = ExprToLoc.find(&ignoreCFGOmittedNodes(E));
-  return It == ExprToLoc.end() ? nullptr : &*It->second;
+  setStorageLocationInternal(E, Loc);
 }
 
 StorageLocation *Environment::getStorageLocationStrict(const Expr &E) const {
   // See comment in `setStorageLocationStrict()`.
   assert(E.isGLValue() ||
          E.getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn));
-  StorageLocation *Loc = getStorageLocation(E, SkipPast::None);
-
-  if (Loc == nullptr)
-    return nullptr;
-
-  return Loc;
+  return getStorageLocationInternal(E);
 }
 
 AggregateStorageLocation *Environment::getThisPointeeStorageLocation() const {
@@ -662,12 +644,11 @@ Environment::getResultObjectLocation(const Expr &RecordPRValue) {
   assert(RecordPRValue.getType()->isRecordType());
   assert(RecordPRValue.isPRValue());
 
-  if (StorageLocation *ExistingLoc =
-          getStorageLocation(RecordPRValue, SkipPast::None))
+  if (StorageLocation *ExistingLoc = getStorageLocationInternal(RecordPRValue))
     return *cast<AggregateStorageLocation>(ExistingLoc);
   auto &Loc = cast<AggregateStorageLocation>(
       DACtx->getStableStorageLocation(RecordPRValue));
-  setStorageLocation(RecordPRValue, Loc);
+  setStorageLocationInternal(RecordPRValue, Loc);
   return Loc;
 }
 
@@ -690,18 +671,18 @@ void Environment::setValueStrict(const Expr &E, Value &Val) {
             cast_or_null<StructValue>(getValue(E)))
       assert(&ExistingVal->getAggregateLoc() == &StructVal->getAggregateLoc());
     if ([[maybe_unused]] StorageLocation *ExistingLoc =
-            getStorageLocation(E, SkipPast::None))
+            getStorageLocationInternal(E))
       assert(ExistingLoc == &StructVal->getAggregateLoc());
     else
-      setStorageLocation(E, StructVal->getAggregateLoc());
+      setStorageLocationInternal(E, StructVal->getAggregateLoc());
     setValue(StructVal->getAggregateLoc(), Val);
     return;
   }
 
-  StorageLocation *Loc = getStorageLocation(E, SkipPast::None);
+  StorageLocation *Loc = getStorageLocationInternal(E);
   if (Loc == nullptr) {
     Loc = &createStorageLocation(E);
-    setStorageLocation(E, *Loc);
+    setStorageLocationInternal(E, *Loc);
   }
   setValue(*Loc, Val);
 }
@@ -717,16 +698,11 @@ Value *Environment::getValue(const ValueDecl &D) const {
   return getValue(*Loc);
 }
 
-Value *Environment::getValue(const Expr &E, SkipPast SP) const {
-  auto *Loc = getStorageLocation(E, SP);
-  if (Loc == nullptr)
+Value *Environment::getValue(const Expr &E) const {
+  auto It = ExprToLoc.find(&ignoreCFGOmittedNodes(E));
+  if (It == ExprToLoc.end())
     return nullptr;
-  return getValue(*Loc);
-}
-
-Value *Environment::getValueStrict(const Expr &E) const {
-  assert(E.isPRValue());
-  return getValue(E);
+  return getValue(*It->second);
 }
 
 Value *Environment::createValue(QualType Type) {
@@ -741,6 +717,18 @@ Value *Environment::createValue(QualType Type) {
   return Val;
 }
 
+void Environment::setStorageLocationInternal(const Expr &E,
+                                             StorageLocation &Loc) {
+  const Expr &CanonE = ignoreCFGOmittedNodes(E);
+  assert(!ExprToLoc.contains(&CanonE));
+  ExprToLoc[&CanonE] = &Loc;
+}
+
+StorageLocation *Environment::getStorageLocationInternal(const Expr &E) const {
+  auto It = ExprToLoc.find(&ignoreCFGOmittedNodes(E));
+  return It == ExprToLoc.end() ? nullptr : &*It->second;
+}
+
 Value *Environment::createValueUnlessSelfReferential(
     QualType Type, llvm::DenseSet<QualType> &Visited, int Depth,
     int &CreatedValuesCount) {


        


More information about the cfe-commits mailing list