mboehme updated this revision to Diff 522591.
mboehme added a comment.
Tweaked preconditions (in comments and code)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150653/new/
https://reviews.llvm.org/D150653
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -135,14 +135,8 @@
// entirety (even if they may share some clauses). So, we need *some* value
// for the condition expression, even if just an atom.
if (Val == nullptr) {
- // FIXME: Consider introducing a helper for this get-or-create pattern.
- auto *Loc = Env.getStorageLocation(Cond, SkipPast::None);
- if (Loc == nullptr) {
- Loc = &Env.createStorageLocation(Cond);
- Env.setStorageLocation(Cond, *Loc);
- }
Val = &Env.makeAtomicBoolValue();
- Env.setValue(*Loc, *Val);
+ Env.setValueStrict(Cond, *Val);
}
bool ConditionValue = true;
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -223,7 +223,7 @@
if (DeclLoc == nullptr)
return;
- Env.setStorageLocation(*S, *DeclLoc);
+ Env.setStorageLocationStrict(*S, *DeclLoc);
}
void VisitDeclStmt(const DeclStmt *S) {
@@ -343,15 +343,13 @@
// This cast creates a new, boolean value from the integral value. We
// model that with a fresh value in the environment, unless it's already a
// boolean.
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocation(*S, Loc);
- if (auto *SubExprVal = dyn_cast_or_null<BoolValue>(
- Env.getValue(*SubExpr, SkipPast::Reference)))
- Env.setValue(Loc, *SubExprVal);
+ if (auto *SubExprVal =
+ dyn_cast_or_null<BoolValue>(Env.getValueStrict(*SubExpr)))
+ Env.setValueStrict(*S, *SubExprVal);
else
// FIXME: If integer modeling is added, then update this code to create
// the boolean based on the integer model.
- Env.setValue(Loc, Env.makeAtomicBoolValue());
+ Env.setValueStrict(*S, Env.makeAtomicBoolValue());
break;
}
@@ -425,29 +423,22 @@
switch (S->getOpcode()) {
case UO_Deref: {
const auto *SubExprVal =
- cast_or_null<PointerValue>(Env.getValue(*SubExpr, SkipPast::None));
+ cast_or_null<PointerValue>(Env.getValueStrict(*SubExpr));
if (SubExprVal == nullptr)
break;
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocation(*S, Loc);
- Env.setValue(Loc,
- Env.create<ReferenceValue>(SubExprVal->getPointeeLoc()));
+ Env.setStorageLocationStrict(*S, SubExprVal->getPointeeLoc());
break;
}
case UO_AddrOf: {
// Do not form a pointer to a reference. If `SubExpr` is assigned a
// `ReferenceValue` then form a value that points to the location of its
// pointee.
- StorageLocation *PointeeLoc =
- Env.getStorageLocation(*SubExpr, SkipPast::Reference);
+ StorageLocation *PointeeLoc = Env.getStorageLocationStrict(*SubExpr);
if (PointeeLoc == nullptr)
break;
- auto &PointerLoc = Env.createStorageLocation(*S);
- auto &PointerVal = Env.create<PointerValue>(*PointeeLoc);
- Env.setStorageLocation(*S, PointerLoc);
- Env.setValue(PointerLoc, PointerVal);
+ Env.setValueStrict(*S, Env.create<PointerValue>(*PointeeLoc));
break;
}
case UO_LNot: {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -623,6 +623,16 @@
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.
@@ -630,6 +640,21 @@
return It == ExprToLoc.end() ? nullptr : &skip(*It->second, SP);
}
+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;
+
+ if (auto *RefVal = dyn_cast_or_null<ReferenceValue>(getValue(*Loc)))
+ return &RefVal->getReferentLoc();
+
+ return Loc;
+}
+
StorageLocation *Environment::getThisPointeeStorageLocation() const {
return ThisPointeeLoc;
}
@@ -675,6 +700,18 @@
}
}
+void Environment::setValueStrict(const Expr &E, Value &Val) {
+ assert(E.isPRValue());
+ assert(!isa<ReferenceValue>(Val));
+
+ StorageLocation *Loc = getStorageLocation(E, SkipPast::None);
+ if (Loc == nullptr) {
+ Loc = &createStorageLocation(E);
+ setStorageLocation(E, *Loc);
+ }
+ setValue(*Loc, Val);
+}
+
Value *Environment::getValue(const StorageLocation &Loc) const {
auto It = LocToVal.find(&Loc);
return It == LocToVal.end() ? nullptr : It->second;
@@ -694,6 +731,15 @@
return getValue(*Loc);
}
+Value *Environment::getValueStrict(const Expr &E) const {
+ assert(E.isPRValue());
+ Value *Val = getValue(E, SkipPast::None);
+
+ assert(Val == nullptr || !isa<ReferenceValue>(Val));
+
+ return Val;
+}
+
Value *Environment::createValue(QualType Type) {
llvm::DenseSet<QualType> Visited;
int CreatedValuesCount = 0;
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -270,16 +270,54 @@
/// 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, applying
/// the `SP` policy for skipping past indirections, or null if `E` isn't
/// assigned a storage location in the environment.
+ ///
+ /// 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.
@@ -305,6 +343,25 @@
/// Assigns `Val` as the value of `Loc` in the environment.
void setValue(const StorageLocation &Loc, Value &Val);
+ /// Assigns `Val` as the value of the prvalue `E` in the environment.
+ ///
+ /// If `E` is not yet associated with a storage location, associates it with
+ /// a newly created storage location. In any case, associates the storage
+ /// location of `E` with `Val`.
+ ///
+ /// Once the migration to strict handling of value categories is complete
+ /// (see https://discourse.llvm.org/t/70086), this function will be renamed to
+ /// `setValue()`. At this point, prvalue expressions will be associated
+ /// directly with `Value`s, and the legacy behavior of associating prvalue
+ /// expressions with storage locations (as described above) will be
+ /// eliminated.
+ ///
+ /// Requirements:
+ ///
+ /// `E` must be a prvalue
+ /// `Val` must not be a `ReferenceValue`
+ void setValueStrict(const Expr &E, Value &Val);
+
/// Returns the value assigned to `Loc` in the environment or null if `Loc`
/// isn't assigned a value in the environment.
Value *getValue(const StorageLocation &Loc) const;
@@ -315,8 +372,25 @@
/// Equivalent to `getValue(getStorageLocation(E, SP), SkipPast::None)` if `E`
/// is assigned a storage location in the environment, otherwise returns null.
+ ///
+ /// This function is deprecated; prefer `getValueStrict()`. For details, see
+ /// https://discourse.llvm.org/t/70086.
Value *getValue(const Expr &E, SkipPast SP) 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 the preferred alternative to
+ /// `getValue(const Expr &, SkipPast)`. Once the migration to strict handling
+ /// of value categories is complete (see https://discourse.llvm.org/t/70086),
+ /// `getValue()` will be removed and this function will be renamed to
+ /// `getValue()`.
+ ///
+ /// Requirements:
+ ///
+ /// `E` must be a prvalue
+ Value *getValueStrict(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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
