[clang] 745a957 - [clang][dataflow] Add `create<T>()` methods to `Environment` and `DataflowAnalysisContext`.
Martin Braenne via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 4 00:13:53 PDT 2023
Author: Martin Braenne
Date: 2023-04-04T07:13:44Z
New Revision: 745a957f9dc562477cbe587fb3fa8305713b51b3
URL: https://github.com/llvm/llvm-project/commit/745a957f9dc562477cbe587fb3fa8305713b51b3
DIFF: https://github.com/llvm/llvm-project/commit/745a957f9dc562477cbe587fb3fa8305713b51b3.diff
LOG: [clang][dataflow] Add `create<T>()` methods to `Environment` and `DataflowAnalysisContext`.
These methods provide a less verbose way of allocating `StorageLocation`s and
`Value`s than the existing `takeOwnership(make_unique(...))` pattern.
In addition, because allocation of `StorageLocation`s and `Value`s now happens
within the `DataflowAnalysisContext`, the `create<T>()` open up the possibility
of using `BumpPtrAllocator` to allocate these objects if it turns out this
helps performance.
Reviewed By: ymandel, xazax.hun, gribozavr2
Differential Revision: https://reviews.llvm.org/D147302
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index a044f477ce1b..31bd9809a72a 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -86,14 +86,51 @@ class DataflowAnalysisContext {
/*Logger=*/nullptr});
~DataflowAnalysisContext();
+ /// Creates a `T` (some subclass of `StorageLocation`), forwarding `args` to
+ /// the constructor, and returns a reference to it.
+ ///
+ /// The `DataflowAnalysisContext` takes ownership of the created object. The
+ /// object will be destroyed when the `DataflowAnalysisContext` is destroyed.
+ template <typename T, typename... Args>
+ std::enable_if_t<std::is_base_of<StorageLocation, T>::value, T &>
+ create(Args &&...args) {
+ // Note: If allocation of individual `StorageLocation`s turns out to be
+ // costly, consider creating specializations of `create<T>` for commonly
+ // used `StorageLocation` subclasses and make them use a `BumpPtrAllocator`.
+ return *cast<T>(
+ Locs.emplace_back(std::make_unique<T>(std::forward<Args>(args)...))
+ .get());
+ }
+
+ /// Creates a `T` (some subclass of `Value`), forwarding `args` to the
+ /// constructor, and returns a reference to it.
+ ///
+ /// The `DataflowAnalysisContext` takes ownership of the created object. The
+ /// object will be destroyed when the `DataflowAnalysisContext` is destroyed.
+ template <typename T, typename... Args>
+ std::enable_if_t<std::is_base_of<Value, T>::value, T &>
+ create(Args &&...args) {
+ // Note: If allocation of individual `Value`s turns out to be costly,
+ // consider creating specializations of `create<T>` for commonly used
+ // `Value` subclasses and make them use a `BumpPtrAllocator`.
+ return *cast<T>(
+ Vals.emplace_back(std::make_unique<T>(std::forward<Args>(args)...))
+ .get());
+ }
+
/// Takes ownership of `Loc` and returns a reference to it.
///
+ /// This function is deprecated. Instead of
+ /// `takeOwnership(std::make_unique<SomeStorageLocation>(args))`, prefer
+ /// `create<SomeStorageLocation>(args)`.
+ ///
/// Requirements:
///
/// `Loc` must not be null.
template <typename T>
- std::enable_if_t<std::is_base_of<StorageLocation, T>::value, T &>
- takeOwnership(std::unique_ptr<T> Loc) {
+ LLVM_DEPRECATED("use create<T> instead", "")
+ std::enable_if_t<std::is_base_of<StorageLocation, T>::value,
+ T &> takeOwnership(std::unique_ptr<T> Loc) {
assert(Loc != nullptr);
Locs.push_back(std::move(Loc));
return *cast<T>(Locs.back().get());
@@ -101,12 +138,17 @@ class DataflowAnalysisContext {
/// Takes ownership of `Val` and returns a reference to it.
///
+ /// This function is deprecated. Instead of
+ /// `takeOwnership(std::make_unique<SomeValue>(args))`, prefer
+ /// `create<SomeValue>(args)`.
+ ///
/// Requirements:
///
/// `Val` must not be null.
template <typename T>
- std::enable_if_t<std::is_base_of<Value, T>::value, T &>
- takeOwnership(std::unique_ptr<T> Val) {
+ LLVM_DEPRECATED("use create<T> instead", "")
+ std::enable_if_t<std::is_base_of<Value, T>::value, T &> takeOwnership(
+ std::unique_ptr<T> Val) {
assert(Val != nullptr);
Vals.push_back(std::move(Val));
return *cast<T>(Vals.back().get());
@@ -170,9 +212,9 @@ class DataflowAnalysisContext {
}
/// Creates an atomic boolean value.
- AtomicBoolValue &createAtomicBoolValue() {
- return takeOwnership(std::make_unique<AtomicBoolValue>());
- }
+ LLVM_DEPRECATED("use create<AtomicBoolValue> instead",
+ "create<AtomicBoolValue>")
+ AtomicBoolValue &createAtomicBoolValue() { return create<AtomicBoolValue>(); }
/// Creates a Top value for booleans. Each instance is unique and can be
/// assigned a distinct truth value during solving.
@@ -182,9 +224,8 @@ class DataflowAnalysisContext {
/// implementation so that `Top iff Top` has a consistent meaning, regardless
/// of the identity of `Top`. Moreover, I think the meaning should be
/// `false`.
- TopBoolValue &createTopBoolValue() {
- return takeOwnership(std::make_unique<TopBoolValue>());
- }
+ LLVM_DEPRECATED("use create<TopBoolValue> instead", "create<TopBoolValue>")
+ TopBoolValue &createTopBoolValue() { return create<TopBoolValue>(); }
/// Returns a boolean value that represents the conjunction of `LHS` and
/// `RHS`. Subsequent calls with the same arguments, regardless of their
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index b4ae172e3fa2..ce11fc1c2b2d 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -319,27 +319,59 @@ class Environment {
/// is assigned a storage location in the environment, otherwise returns null.
Value *getValue(const Expr &E, SkipPast SP) const;
+ /// Creates a `T` (some subclass of `StorageLocation`), forwarding `args` to
+ /// the constructor, and returns a reference to it.
+ ///
+ /// The analysis context takes ownership of the created object. The object
+ /// will be destroyed when the analysis context is destroyed.
+ template <typename T, typename... Args>
+ std::enable_if_t<std::is_base_of<StorageLocation, T>::value, T &>
+ create(Args &&...args) {
+ return DACtx->create<T>(std::forward<Args>(args)...);
+ }
+
+ /// Creates a `T` (some subclass of `Value`), forwarding `args` to the
+ /// constructor, and returns a reference to it.
+ ///
+ /// The analysis context takes ownership of the created object. The object
+ /// will be destroyed when the analysis context is destroyed.
+ template <typename T, typename... Args>
+ std::enable_if_t<std::is_base_of<Value, T>::value, T &>
+ create(Args &&...args) {
+ return DACtx->create<T>(std::forward<Args>(args)...);
+ }
+
/// Transfers ownership of `Loc` to the analysis context and returns a
/// reference to it.
///
+ /// This function is deprecated. Instead of
+ /// `takeOwnership(std::make_unique<SomeStorageLocation>(args))`, prefer
+ /// `create<SomeStorageLocation>(args)`.
+ ///
/// Requirements:
///
/// `Loc` must not be null.
template <typename T>
- std::enable_if_t<std::is_base_of<StorageLocation, T>::value, T &>
- takeOwnership(std::unique_ptr<T> Loc) {
+ LLVM_DEPRECATED("use create<T> instead", "")
+ std::enable_if_t<std::is_base_of<StorageLocation, T>::value,
+ T &> takeOwnership(std::unique_ptr<T> Loc) {
return DACtx->takeOwnership(std::move(Loc));
}
/// Transfers ownership of `Val` to the analysis context and returns a
/// reference to it.
///
+ /// This function is deprecated. Instead of
+ /// `takeOwnership(std::make_unique<SomeValue>(args))`, prefer
+ /// `create<SomeValue>(args)`.
+ ///
/// Requirements:
///
/// `Val` must not be null.
template <typename T>
- std::enable_if_t<std::is_base_of<Value, T>::value, T &>
- takeOwnership(std::unique_ptr<T> Val) {
+ LLVM_DEPRECATED("use create<T> instead", "")
+ std::enable_if_t<std::is_base_of<Value, T>::value, T &> takeOwnership(
+ std::unique_ptr<T> Val) {
return DACtx->takeOwnership(std::move(Val));
}
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index 57169baccbd4..5a49ef195c0b 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -59,10 +59,9 @@ StorageLocation &DataflowAnalysisContext::createStorageLocation(QualType Type) {
: getReferencedFields(Type);
for (const FieldDecl *Field : Fields)
FieldLocs.insert({Field, &createStorageLocation(Field->getType())});
- return takeOwnership(
- std::make_unique<AggregateStorageLocation>(Type, std::move(FieldLocs)));
+ return create<AggregateStorageLocation>(Type, std::move(FieldLocs));
}
- return takeOwnership(std::make_unique<ScalarStorageLocation>(Type));
+ return create<ScalarStorageLocation>(Type);
}
StorageLocation &
@@ -90,8 +89,7 @@ DataflowAnalysisContext::getOrCreateNullPointerValue(QualType PointeeType) {
auto Res = NullPointerVals.try_emplace(CanonicalPointeeType, nullptr);
if (Res.second) {
auto &PointeeLoc = createStorageLocation(CanonicalPointeeType);
- Res.first->second =
- &takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
+ Res.first->second = &create<PointerValue>(PointeeLoc);
}
return *Res.first->second;
}
@@ -112,8 +110,7 @@ BoolValue &DataflowAnalysisContext::getOrCreateConjunction(BoolValue &LHS,
auto Res = ConjunctionVals.try_emplace(makeCanonicalBoolValuePair(LHS, RHS),
nullptr);
if (Res.second)
- Res.first->second =
- &takeOwnership(std::make_unique<ConjunctionValue>(LHS, RHS));
+ Res.first->second = &create<ConjunctionValue>(LHS, RHS);
return *Res.first->second;
}
@@ -125,15 +122,14 @@ BoolValue &DataflowAnalysisContext::getOrCreateDisjunction(BoolValue &LHS,
auto Res = DisjunctionVals.try_emplace(makeCanonicalBoolValuePair(LHS, RHS),
nullptr);
if (Res.second)
- Res.first->second =
- &takeOwnership(std::make_unique<DisjunctionValue>(LHS, RHS));
+ Res.first->second = &create<DisjunctionValue>(LHS, RHS);
return *Res.first->second;
}
BoolValue &DataflowAnalysisContext::getOrCreateNegation(BoolValue &Val) {
auto Res = NegationVals.try_emplace(&Val, nullptr);
if (Res.second)
- Res.first->second = &takeOwnership(std::make_unique<NegationValue>(Val));
+ Res.first->second = &create<NegationValue>(Val);
return *Res.first->second;
}
@@ -144,8 +140,7 @@ BoolValue &DataflowAnalysisContext::getOrCreateImplication(BoolValue &LHS,
auto Res = ImplicationVals.try_emplace(std::make_pair(&LHS, &RHS), nullptr);
if (Res.second)
- Res.first->second =
- &takeOwnership(std::make_unique<ImplicationValue>(LHS, RHS));
+ Res.first->second = &create<ImplicationValue>(LHS, RHS);
return *Res.first->second;
}
@@ -157,8 +152,7 @@ BoolValue &DataflowAnalysisContext::getOrCreateIff(BoolValue &LHS,
auto Res = BiconditionalVals.try_emplace(makeCanonicalBoolValuePair(LHS, RHS),
nullptr);
if (Res.second)
- Res.first->second =
- &takeOwnership(std::make_unique<BiconditionalValue>(LHS, RHS));
+ Res.first->second = &create<BiconditionalValue>(LHS, RHS);
return *Res.first->second;
}
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index fbb8d8ab7edd..faeabdcbeed6 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -359,7 +359,7 @@ void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
QualType ParamType = Param->getType();
if (ParamType->isReferenceType()) {
- auto &Val = takeOwnership(std::make_unique<ReferenceValue>(*ArgLoc));
+ auto &Val = create<ReferenceValue>(*ArgLoc);
setValue(Loc, Val);
} else if (auto *ArgVal = getValue(*ArgLoc)) {
setValue(Loc, *ArgVal);
@@ -685,7 +685,7 @@ Value *Environment::createValueUnlessSelfReferential(
// with integers, and so distinguishing them serves no purpose, but could
// prevent convergence.
CreatedValuesCount++;
- return &takeOwnership(std::make_unique<IntegerValue>());
+ return &create<IntegerValue>();
}
if (Type->isReferenceType()) {
@@ -702,7 +702,7 @@ Value *Environment::createValueUnlessSelfReferential(
setValue(PointeeLoc, *PointeeVal);
}
- return &takeOwnership(std::make_unique<ReferenceValue>(PointeeLoc));
+ return &create<ReferenceValue>(PointeeLoc);
}
if (Type->isPointerType()) {
@@ -719,7 +719,7 @@ Value *Environment::createValueUnlessSelfReferential(
setValue(PointeeLoc, *PointeeVal);
}
- return &takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
+ return &create<PointerValue>(PointeeLoc);
}
if (Type->isStructureOrClassType() || Type->isUnionType()) {
@@ -739,8 +739,7 @@ Value *Environment::createValueUnlessSelfReferential(
Visited.erase(FieldType.getCanonicalType());
}
- return &takeOwnership(
- std::make_unique<StructValue>(std::move(FieldValues)));
+ return &create<StructValue>(std::move(FieldValues));
}
return nullptr;
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index aebe326b024f..a91ec5d6ad9c 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -221,9 +221,9 @@ void setHasValue(Value &OptionalVal, BoolValue &HasValueVal) {
/// Creates a symbolic value for an `optional` value using `HasValueVal` as the
/// symbolic value of its "has_value" property.
StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
- auto OptionalVal = std::make_unique<StructValue>();
- setHasValue(*OptionalVal, HasValueVal);
- return Env.takeOwnership(std::move(OptionalVal));
+ auto &OptionalVal = Env.create<StructValue>();
+ setHasValue(OptionalVal, HasValueVal);
+ return OptionalVal;
}
/// Returns the symbolic value that represents the "has_value" property of the
@@ -312,8 +312,8 @@ StorageLocation *maybeInitializeOptionalValueMember(QualType Q,
return nullptr;
auto &ValueLoc = Env.createStorageLocation(Ty);
Env.setValue(ValueLoc, *ValueVal);
- auto ValueRef = std::make_unique<ReferenceValue>(ValueLoc);
- OptionalVal.setProperty("value", Env.takeOwnership(std::move(ValueRef)));
+ auto &ValueRef = Env.create<ReferenceValue>(ValueLoc);
+ OptionalVal.setProperty("value", ValueRef);
return &ValueLoc;
}
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index d255d27e52c4..aa8fe907b167 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -237,7 +237,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
Env.setStorageLocation(*S, *DeclLoc);
} else {
auto &Loc = Env.createStorageLocation(*S);
- auto &Val = Env.takeOwnership(std::make_unique<ReferenceValue>(*DeclLoc));
+ auto &Val = Env.create<ReferenceValue>(*DeclLoc);
Env.setStorageLocation(*S, Loc);
Env.setValue(Loc, Val);
}
@@ -276,8 +276,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
// FIXME: reuse the ReferenceValue instead of creating a new one.
if (auto *InitExprLoc =
Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
- auto &Val =
- Env.takeOwnership(std::make_unique<ReferenceValue>(*InitExprLoc));
+ auto &Val = Env.create<ReferenceValue>(*InitExprLoc);
Env.setValue(Loc, Val);
}
} else if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
@@ -423,8 +422,8 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
- Env.setValue(Loc, Env.takeOwnership(std::make_unique<ReferenceValue>(
- SubExprVal->getPointeeLoc())));
+ Env.setValue(Loc,
+ Env.create<ReferenceValue>(SubExprVal->getPointeeLoc()));
break;
}
case UO_AddrOf: {
@@ -437,8 +436,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
break;
auto &PointerLoc = Env.createStorageLocation(*S);
- auto &PointerVal =
- Env.takeOwnership(std::make_unique<PointerValue>(*PointeeLoc));
+ auto &PointerVal = Env.create<PointerValue>(*PointeeLoc);
Env.setStorageLocation(*S, PointerLoc);
Env.setValue(PointerLoc, PointerVal);
break;
@@ -468,8 +466,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
- Env.setValue(Loc, Env.takeOwnership(
- std::make_unique<PointerValue>(*ThisPointeeLoc)));
+ Env.setValue(Loc, Env.create<PointerValue>(*ThisPointeeLoc));
}
void VisitReturnStmt(const ReturnStmt *S) {
@@ -523,8 +520,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
} else {
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
- Env.setValue(Loc, Env.takeOwnership(
- std::make_unique<ReferenceValue>(*VarDeclLoc)));
+ Env.setValue(Loc, Env.create<ReferenceValue>(*VarDeclLoc));
}
return;
}
@@ -558,8 +554,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
} else {
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
- Env.setValue(
- Loc, Env.takeOwnership(std::make_unique<ReferenceValue>(MemberLoc)));
+ Env.setValue(Loc, Env.create<ReferenceValue>(MemberLoc));
}
}
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 8e821e5a75d4..ad250c432e2c 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -312,8 +312,7 @@ void builtinTransferInitializer(const CFGInitializer &Elt,
if (Member->getType()->isReferenceType()) {
auto &MemberLoc = ThisLoc.getChild(*Member);
- Env.setValue(MemberLoc, Env.takeOwnership(std::make_unique<ReferenceValue>(
- *InitStmtLoc)));
+ Env.setValue(MemberLoc, Env.create<ReferenceValue>(*InitStmtLoc));
} else {
auto &MemberLoc = ThisLoc.getChild(*Member);
Env.setValue(MemberLoc, *InitStmtVal);
More information about the cfe-commits
mailing list