Skip to content

Commit

Permalink
[clang][dataflow] Remove deprecated accessors as well as SkipPast.
Browse files Browse the repository at this point in the history 
Depends On D156672

Reviewed By: ymandel, xazax.hun

Differential Revision: https://reviews.llvm.org/D156673
  • Loading branch information
@martinboehme
martinboehme committed Jul 31, 2023
1 parent f76f667 commit 17ba278
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 94 deletions.
67 changes: 9 additions & 58 deletions clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -280,53 +267,22 @@ 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.
///
/// 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;
Expand Down Expand Up @@ -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?

Expand Down Expand Up @@ -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.
///
Expand Down
60 changes: 24 additions & 36 deletions clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -618,39 +618,21 @@ 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,
// 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 {
Expand All @@ -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;
}

Expand All @@ -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);
}
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down

0 comments on commit 17ba278

Please sign in to comment.