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

[mboehme]

Summary

Depends On D156672

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

 #include "llvm/Support/ErrorHandling.h"
 #include <memory>
 #include <type_traits>
 #include <utility>
 
 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,
   Different,
   Unknown,
 };
 
 /// Holds the state of the program (store and heap) at a given program point.
public:
 
   /// Returns the storage location assigned to `D` in the environment, or null
   /// if `D` isn't assigned a storage location in the environment.
   ///
   /// Note that if `D` has reference type, the storage location that is returned
   /// 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.
   ///
   /// 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;
 
   /// Returns the storage location assigned to the `this` pointee in the
   /// environment or null if the `this` pointee has no assigned storage location
   /// in the environment.
   AggregateStorageLocation *getThisPointeeStorageLocation() const;
public:
   Value *getValue(const StorageLocation &Loc) const;
 
   /// Equivalent to `getValue(getStorageLocation(D))` if `D` is assigned a
   /// storage location in the environment, otherwise returns null.
   Value *getValue(const ValueDecl &D) const;
 
   /// 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?
 
   /// Creates a `T` (some subclass of `Value`), forwarding `args` to the
   /// constructor, and returns a reference to it.
   ///
   /// The analysis context takes ownership of the created object. The object
   /// will be destroyed when the analysis context is destroyed.
public:
 
   LLVM_DUMP_METHOD void dump() const;
   LLVM_DUMP_METHOD void dump(raw_ostream &OS) const;
 
 private:
   // 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.
   ///
   /// Recursively initializes storage locations and values until it sees a
   /// self-referential pointer or reference type. `Visited` is used to track
   /// which types appeared in the reference/pointer chain in order to avoid
   /// creating a cyclic dependency with self-referential pointers/references.
   ///

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

StorageLocation *Environment::getStorageLocation(const ValueDecl &D) const {
   if (It == DeclToLoc.end())
     return nullptr;
 
   StorageLocation *Loc = It->second;
 
   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,
   // but we still want to be able to associate a `StorageLocation` with them,
   // 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 {
   return ThisPointeeLoc;
 }
 
 AggregateStorageLocation &
 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;
 }
 
 PointerValue &Environment::getOrCreateNullPointerValue(QualType PointeeType) {
   return DACtx->getOrCreateNullPointerValue(PointeeType);
 }
 
 void Environment::setValue(const StorageLocation &Loc, Value &Val) {
   assert(!isa<StructValue>(&Val) ||
          &cast<StructValue>(&Val)->getAggregateLoc() == &Loc);
 
   LocToVal[&Loc] = &Val;
 }
 
 void Environment::setValueStrict(const Expr &E, Value &Val) {
   assert(E.isPRValue());
 
   if (auto *StructVal = dyn_cast<StructValue>(&Val)) {
     if ([[maybe_unused]] auto *ExistingVal =
             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);
 }
 
 Value *Environment::getValue(const StorageLocation &Loc) const {
   return LocToVal.lookup(&Loc);
 }
 
 Value *Environment::getValue(const ValueDecl &D) const {
   auto *Loc = getStorageLocation(D);
   if (Loc == nullptr)
     return nullptr;
   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) {
   llvm::DenseSet<QualType> Visited;
   int CreatedValuesCount = 0;
   Value *Val = createValueUnlessSelfReferential(Type, Visited, /*Depth=*/0,
                                                 CreatedValuesCount);
   if (CreatedValuesCount > MaxCompositeValueSize) {
     llvm::errs() << "Attempting to initialize a huge value of type: " << Type
                  << '\n';
   }
   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) {
   assert(!Type.isNull());
   assert(!Type->isReferenceType());
 
   // Allow unlimited fields at depth 1; only cap at deeper nesting levels.
   if ((Depth > 1 && CreatedValuesCount > MaxCompositeValueSize) ||