diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h --- 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 @@ /// 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 @@ /// 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 @@ /// 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 @@ // 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 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -618,12 +618,6 @@ 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 @@ // 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 @@ assert(RecordPRValue.getType()->isRecordType()); assert(RecordPRValue.isPRValue()); - if (StorageLocation *ExistingLoc = - getStorageLocation(RecordPRValue, SkipPast::None)) + if (StorageLocation *ExistingLoc = getStorageLocationInternal(RecordPRValue)) return *cast(ExistingLoc); auto &Loc = cast( DACtx->getStableStorageLocation(RecordPRValue)); - setStorageLocation(RecordPRValue, Loc); + setStorageLocationInternal(RecordPRValue, Loc); return Loc; } @@ -690,18 +671,18 @@ cast_or_null(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 @@ 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 @@ 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 &Visited, int Depth, int &CreatedValuesCount) {