[clang] 6d76854 - [clang][dataflow] Add `DataflowEnvironment::createObject()`.
Martin Braenne via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 17 00:26:23 PDT 2023
Author: Martin Braenne
Date: 2023-07-17T07:26:10Z
New Revision: 6d768548ecc0ca37026986f397392c1d0ace9736
URL: https://github.com/llvm/llvm-project/commit/6d768548ecc0ca37026986f397392c1d0ace9736
DIFF: https://github.com/llvm/llvm-project/commit/6d768548ecc0ca37026986f397392c1d0ace9736.diff
LOG: [clang][dataflow] Add `DataflowEnvironment::createObject()`.
This consolidates the code used in various places to initialize objects (usually for variables) into one central location.
It will also help reduce the number of changes needed when we make the upcoming API changes to `AggregateStorageLocation` and `StructValue`.
Depends On D155074
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D155075
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 07af485e786c81..dc82ccea4a27d4 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -394,6 +394,32 @@ class Environment {
/// `Type` must not be null.
Value *createValue(QualType Type);
+ /// Creates an object (i.e. a storage location with an associated value) of
+ /// type `Ty`. If `InitExpr` is non-null and has a value associated with it,
+ /// initializes the object with this value. Otherwise, initializes the object
+ /// with a value created using `createValue()`.
+ StorageLocation &createObject(QualType Ty, const Expr *InitExpr = nullptr) {
+ return createObjectInternal(nullptr, Ty, InitExpr);
+ }
+
+ /// Creates an object for the variable declaration `D`. If `D` has an
+ /// initializer and this initializer is associated with a value, initializes
+ /// the object with this value. Otherwise, initializes the object with a
+ /// value created using `createValue()`. Uses the storage location returned by
+ /// `DataflowAnalysisContext::getStableStorageLocation(D)`.
+ StorageLocation &createObject(const VarDecl &D) {
+ return createObjectInternal(&D, D.getType(), D.getInit());
+ }
+
+ /// Creates an object for the variable declaration `D`. If `InitExpr` is
+ /// non-null and has a value associated with it, initializes the object with
+ /// this value. Otherwise, initializes the object with a value created using
+ /// `createValue()`. Uses the storage location returned by
+ /// `DataflowAnalysisContext::getStableStorageLocation(D)`.
+ StorageLocation &createObject(const VarDecl &D, const Expr *InitExpr) {
+ return createObjectInternal(&D, D.getType(), InitExpr);
+ }
+
/// Assigns `Val` as the value of `Loc` in the environment.
void setValue(const StorageLocation &Loc, Value &Val);
@@ -592,6 +618,11 @@ class Environment {
llvm::DenseSet<QualType> &Visited,
int Depth, int &CreatedValuesCount);
+ /// Shared implementation of `createObject()` overloads.
+ /// `D` and `InitExpr` may be null.
+ StorageLocation &createObjectInternal(const VarDecl *D, QualType Ty,
+ const Expr *InitExpr);
+
StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index 531115efa64f94..bb1c2c7a1de10d 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -72,7 +72,7 @@ StorageLocation &
DataflowAnalysisContext::getStableStorageLocation(const VarDecl &D) {
if (auto *Loc = getStorageLocation(D))
return *Loc;
- auto &Loc = createStorageLocation(D.getType());
+ auto &Loc = createStorageLocation(D.getType().getNonReferenceType());
setStorageLocation(D, Loc);
return Loc;
}
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index a49c529c37a46b..fdd6b5959cd722 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -260,10 +260,8 @@ void Environment::initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl) {
for (const VarDecl *D : Vars) {
if (getStorageLocation(*D) != nullptr)
continue;
- auto &Loc = createStorageLocation(D->getType().getNonReferenceType());
- setStorageLocation(*D, Loc);
- if (auto *Val = createValue(D->getType().getNonReferenceType()))
- setValue(Loc, *Val);
+
+ setStorageLocation(*D, createObject(*D));
}
for (const FunctionDecl *FD : Funcs) {
@@ -296,16 +294,7 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
for (const auto *ParamDecl : FuncDecl->parameters()) {
assert(ParamDecl != nullptr);
- // References aren't objects, so the reference itself doesn't have a
- // storage location. Instead, the storage location for a reference refers
- // directly to an object of the referenced type -- so strip off any
- // reference from the type.
- auto &ParamLoc =
- createStorageLocation(ParamDecl->getType().getNonReferenceType());
- setStorageLocation(*ParamDecl, ParamLoc);
- if (Value *ParamVal =
- createValue(ParamDecl->getType().getNonReferenceType()))
- setValue(ParamLoc, *ParamVal);
+ setStorageLocation(*ParamDecl, createObject(*ParamDecl, nullptr));
}
}
@@ -382,27 +371,8 @@ void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
// overloaded operators implemented as member functions, and parameter packs.
for (unsigned ArgIndex = 0; ArgIndex < Args.size(); ++ParamIt, ++ArgIndex) {
assert(ParamIt != FuncDecl->param_end());
-
- const Expr *Arg = Args[ArgIndex];
- auto *ArgLoc = getStorageLocation(*Arg, SkipPast::Reference);
- if (ArgLoc == nullptr)
- continue;
-
const VarDecl *Param = *ParamIt;
-
- QualType ParamType = Param->getType();
- if (ParamType->isReferenceType()) {
- setStorageLocation(*Param, *ArgLoc);
- } else {
- auto &Loc = createStorageLocation(*Param);
- setStorageLocation(*Param, Loc);
-
- if (auto *ArgVal = getValue(*ArgLoc)) {
- setValue(Loc, *ArgVal);
- } else if (Value *Val = createValue(ParamType)) {
- setValue(Loc, *Val);
- }
- }
+ setStorageLocation(*Param, createObject(*Param, Args[ArgIndex]));
}
}
@@ -872,6 +842,54 @@ Value *Environment::createValueUnlessSelfReferential(
return nullptr;
}
+StorageLocation &Environment::createObjectInternal(const VarDecl *D,
+ QualType Ty,
+ const Expr *InitExpr) {
+ if (Ty->isReferenceType()) {
+ // Although variables of reference type always need to be initialized, it
+ // can happen that we can't see the initializer, so `InitExpr` may still
+ // be null.
+ if (InitExpr) {
+ if (auto *InitExprLoc =
+ getStorageLocation(*InitExpr, SkipPast::Reference))
+ return *InitExprLoc;
+ }
+
+ // Even though we have an initializer, we might not get an
+ // InitExprLoc, for example if the InitExpr is a CallExpr for which we
+ // don't have a function body. In this case, we just invent a storage
+ // location and value -- it's the best we can do.
+ return createObjectInternal(D, Ty.getNonReferenceType(), nullptr);
+ }
+
+ Value *Val = nullptr;
+ if (InitExpr)
+ // In the (few) cases where an expression is intentionally
+ // "uninterpreted", `InitExpr` is not associated with a value. There are
+ // two ways to handle this situation: propagate the status, so that
+ // uninterpreted initializers result in uninterpreted variables, or
+ // provide a default value. We choose the latter so that later refinements
+ // of the variable can be used for reasoning about the surrounding code.
+ // For this reason, we let this case be handled by the `createValue()`
+ // call below.
+ //
+ // FIXME. If and when we interpret all language cases, change this to
+ // assert that `InitExpr` is interpreted, rather than supplying a
+ // default value (assuming we don't update the environment API to return
+ // references).
+ Val = getValueStrict(*InitExpr);
+ if (!Val)
+ Val = createValue(Ty);
+
+ StorageLocation &Loc =
+ D ? createStorageLocation(*D) : createStorageLocation(Ty);
+
+ if (Val)
+ setValue(Loc, *Val);
+
+ return Loc;
+}
+
StorageLocation &Environment::skip(StorageLocation &Loc, SkipPast SP) const {
switch (SP) {
case SkipPast::None:
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 4c97e81184bba3..f51b9f34d98639 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -208,62 +208,15 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (D.hasGlobalStorage())
return;
- if (D.getType()->isReferenceType()) {
- // If this is the holding variable for a `BindingDecl`, we may already
- // have a storage location set up -- so check. (See also explanation below
- // where we process the `BindingDecl`.)
- if (Env.getStorageLocation(D) == nullptr) {
- const Expr *InitExpr = D.getInit();
- assert(InitExpr != nullptr);
- if (auto *InitExprLoc =
- Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
- Env.setStorageLocation(D, *InitExprLoc);
- } else {
- // Even though we have an initializer, we might not get an
- // InitExprLoc, for example if the InitExpr is a CallExpr for which we
- // don't have a function body. In this case, we just invent a storage
- // location and value -- it's the best we can do.
- StorageLocation &Loc =
- Env.createStorageLocation(D.getType().getNonReferenceType());
- Env.setStorageLocation(D, Loc);
- if (Value *Val = Env.createValue(D.getType().getNonReferenceType()))
- Env.setValue(Loc, *Val);
- }
- }
- } else {
- // Not a reference type.
+ // If this is the holding variable for a `BindingDecl`, we may already
+ // have a storage location set up -- so check. (See also explanation below
+ // where we process the `BindingDecl`.)
+ if (D.getType()->isReferenceType() && Env.getStorageLocation(D) != nullptr)
+ return;
- assert(Env.getStorageLocation(D) == nullptr);
- StorageLocation &Loc = Env.createStorageLocation(D);
- Env.setStorageLocation(D, Loc);
+ assert(Env.getStorageLocation(D) == nullptr);
- const Expr *InitExpr = D.getInit();
- if (InitExpr == nullptr) {
- // No initializer expression - associate `Loc` with a new value.
- if (Value *Val = Env.createValue(D.getType()))
- Env.setValue(Loc, *Val);
- return;
- }
-
- if (auto *InitExprVal = Env.getValueStrict(*InitExpr))
- Env.setValue(Loc, *InitExprVal);
-
- if (Env.getValue(Loc) == nullptr) {
- // We arrive here in (the few) cases where an expression is
- // intentionally "uninterpreted". There are two ways to handle this
- // situation: propagate the status, so that uninterpreted initializers
- // result in uninterpreted variables, or provide a default value. We
- // choose the latter so that later refinements of the variable can be
- // used for reasoning about the surrounding code.
- //
- // FIXME. If and when we interpret all language cases, change this to
- // assert that `InitExpr` is interpreted, rather than supplying a
- // default value (assuming we don't update the environment API to return
- // references).
- if (Value *Val = Env.createValue(D.getType()))
- Env.setValue(Loc, *Val);
- }
- }
+ Env.setStorageLocation(D, Env.createObject(D));
// `DecompositionDecl` must be handled after we've interpreted the loc
// itself, because the binding expression refers back to the
More information about the cfe-commits
mailing list