Skip to content

Commit

Permalink
[clang][dataflow][NFC] Eliminate getStorageLocation() / `setStorage…
Browse files Browse the repository at this point in the history
…Location()` in `DataflowAnalysisContext`.

Instead, inline them into the `getStableStorageLocation()` overloads, which is the only place they were called from (and should be called from).

`getStorageLocation()` / `setStorageLocation()` were confusing because neither their name nor their documentation made reference to the fact that the storage location is stable.

It didn't make sense to keep these as private member functions either. The code for the two `getStableStorageLocation()` overloads has become only marginally more complex by inlining these functions, and the `Expr` version is actually more efficient because we only call `ignoreCFGOmittedNodes()` once instead of twice.

Reviewed By: ymandel, xazax.hun

Differential Revision: https://reviews.llvm.org/D158981
  • Loading branch information
martinboehme committed Aug 29, 2023
1 parent 20e6515 commit aef05a1
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,39 +103,6 @@ class DataflowAnalysisContext {
/// Returns a stable storage location for `E`.
StorageLocation &getStableStorageLocation(const Expr &E);

/// Assigns `Loc` as the storage location of `D`.
///
/// Requirements:
///
/// `D` must not be assigned a storage location.
void setStorageLocation(const ValueDecl &D, StorageLocation &Loc) {
assert(!DeclToLoc.contains(&D));
DeclToLoc[&D] = &Loc;
}

/// Returns the storage location assigned to `D` or null if `D` has no
/// assigned storage location.
StorageLocation *getStorageLocation(const ValueDecl &D) const {
return DeclToLoc.lookup(&D);
}

/// Assigns `Loc` as the storage location of `E`.
///
/// Requirements:
///
/// `E` must not be assigned a storage location.
void setStorageLocation(const Expr &E, StorageLocation &Loc) {
const Expr &CanonE = ignoreCFGOmittedNodes(E);
assert(!ExprToLoc.contains(&CanonE));
ExprToLoc[&CanonE] = &Loc;
}

/// Returns the storage location assigned to `E` or null if `E` has no
/// assigned storage location.
StorageLocation *getStorageLocation(const Expr &E) const {
return ExprToLoc.lookup(&ignoreCFGOmittedNodes(E));
}

/// Returns a pointer value that represents a null pointer. Calls with
/// `PointeeType` that are canonically equivalent will return the same result.
/// A null `PointeeType` can be used for the pointee of `std::nullptr_t`.
Expand Down
12 changes: 7 additions & 5 deletions clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,21 @@ StorageLocation &DataflowAnalysisContext::createStorageLocation(QualType Type) {

StorageLocation &
DataflowAnalysisContext::getStableStorageLocation(const VarDecl &D) {
if (auto *Loc = getStorageLocation(D))
if (auto *Loc = DeclToLoc.lookup(&D))
return *Loc;
auto &Loc = createStorageLocation(D.getType().getNonReferenceType());
setStorageLocation(D, Loc);
DeclToLoc[&D] = &Loc;
return Loc;
}

StorageLocation &
DataflowAnalysisContext::getStableStorageLocation(const Expr &E) {
if (auto *Loc = getStorageLocation(E))
const Expr &CanonE = ignoreCFGOmittedNodes(E);

if (auto *Loc = ExprToLoc.lookup(&CanonE))
return *Loc;
auto &Loc = createStorageLocation(E.getType());
setStorageLocation(E, Loc);
auto &Loc = createStorageLocation(CanonE.getType());
ExprToLoc[&CanonE] = &Loc;
return Loc;
}

Expand Down

0 comments on commit aef05a1

Please sign in to comment.