[clang] 49ed5bf - [clang][dataflow] Enable use of synthetic properties on all Value instances.
Dmitri Gribenko via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 8 11:20:31 PDT 2022
Author: Wei Yi Tee
Date: 2022-06-08T20:20:26+02:00
New Revision: 49ed5bf51958aaeb209804da794c85d06207c3ed
URL: https://github.com/llvm/llvm-project/commit/49ed5bf51958aaeb209804da794c85d06207c3ed
DIFF: https://github.com/llvm/llvm-project/commit/49ed5bf51958aaeb209804da794c85d06207c3ed.diff
LOG: [clang][dataflow] Enable use of synthetic properties on all Value instances.
This patch moves the implementation of synthetic properties from the StructValue class into the Value base class so that it can be used across all Value instances.
Reviewed By: gribozavr2, ymandel, sgatev, xazax.hun
Differential Revision: https://reviews.llvm.org/D127196
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/Value.h b/clang/include/clang/Analysis/FlowSensitive/Value.h
index 859cf7ff21b5b..1aedd8a300dd5 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -26,6 +26,9 @@ namespace clang {
namespace dataflow {
/// Base class for all values computed by abstract interpretation.
+///
+/// Don't use `Value` instances by value. All `Value` instances are allocated
+/// and owned by `DataflowAnalysisContext`.
class Value {
public:
enum class Kind {
@@ -48,8 +51,22 @@ class Value {
Kind getKind() const { return ValKind; }
+ /// Returns the value of the synthetic property with the given `Name` or null
+ /// if the property isn't assigned a value.
+ Value *getProperty(llvm::StringRef Name) const {
+ auto It = Properties.find(Name);
+ return It == Properties.end() ? nullptr : It->second;
+ }
+
+ /// Assigns `Val` as the value of the synthetic property with the given
+ /// `Name`.
+ void setProperty(llvm::StringRef Name, Value &Val) {
+ Properties.insert_or_assign(Name, &Val);
+ }
+
private:
Kind ValKind;
+ llvm::StringMap<Value *> Properties;
};
/// Models a boolean.
@@ -215,22 +232,8 @@ class StructValue final : public Value {
/// Assigns `Val` as the child value for `D`.
void setChild(const ValueDecl &D, Value &Val) { Children[&D] = &Val; }
- /// Returns the value of the synthetic property with the given `Name` or null
- /// if the property isn't assigned a value.
- Value *getProperty(llvm::StringRef Name) const {
- auto It = Properties.find(Name);
- return It == Properties.end() ? nullptr : It->second;
- }
-
- /// Assigns `Val` as the value of the synthetic property with the given
- /// `Name`.
- void setProperty(llvm::StringRef Name, Value &Val) {
- Properties.insert_or_assign(Name, &Val);
- }
-
private:
llvm::DenseMap<const ValueDecl *, Value *> Children;
- llvm::StringMap<Value *> Properties;
};
} // namespace dataflow
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 02cf2c75579ff..cf93a07f832df 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -178,9 +178,9 @@ StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
}
/// Returns the symbolic value that represents the "has_value" property of the
-/// optional value `Val`. Returns null if `Val` is null.
-BoolValue *getHasValue(Value *Val) {
- if (auto *OptionalVal = cast_or_null<StructValue>(Val)) {
+/// optional value `OptionalVal`. Returns null if `OptionalVal` is null.
+BoolValue *getHasValue(Value *OptionalVal) {
+ if (OptionalVal) {
return cast<BoolValue>(OptionalVal->getProperty("has_value"));
}
return nullptr;
@@ -221,8 +221,8 @@ int countOptionalWrappers(const ASTContext &ASTCtx, QualType Type) {
void initializeOptionalReference(const Expr *OptionalExpr,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
- if (auto *OptionalVal = cast_or_null<StructValue>(
- State.Env.getValue(*OptionalExpr, SkipPast::Reference))) {
+ if (auto *OptionalVal =
+ State.Env.getValue(*OptionalExpr, SkipPast::Reference)) {
if (OptionalVal->getProperty("has_value") == nullptr) {
OptionalVal->setProperty("has_value", State.Env.makeAtomicBoolValue());
}
@@ -231,8 +231,8 @@ void initializeOptionalReference(const Expr *OptionalExpr,
void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
LatticeTransferState &State) {
- if (auto *OptionalVal = cast_or_null<StructValue>(
- State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer))) {
+ if (auto *OptionalVal =
+ State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer)) {
auto *HasValueVal = getHasValue(OptionalVal);
assert(HasValueVal != nullptr);
diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 2509aa3b9d8dd..594764b4e71bd 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -312,7 +312,7 @@ class SpecialBoolAnalysis
if (const auto *E = selectFirst<CXXConstructExpr>(
"call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S,
getASTContext()))) {
- auto &ConstructorVal = *cast<StructValue>(Env.createValue(E->getType()));
+ auto &ConstructorVal = *Env.createValue(E->getType());
ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(false));
Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
} else if (const auto *E = selectFirst<CXXMemberCallExpr>(
@@ -327,8 +327,7 @@ class SpecialBoolAnalysis
Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
assert(ObjectLoc != nullptr);
- auto &ConstructorVal =
- *cast<StructValue>(Env.createValue(Object->getType()));
+ auto &ConstructorVal = *Env.createValue(Object->getType());
ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(true));
Env.setValue(*ObjectLoc, ConstructorVal);
}
@@ -342,13 +341,11 @@ class SpecialBoolAnalysis
Decl->getName() != "SpecialBool")
return false;
- auto *IsSet1 = cast_or_null<BoolValue>(
- cast<StructValue>(&Val1)->getProperty("is_set"));
+ auto *IsSet1 = cast_or_null<BoolValue>(Val1.getProperty("is_set"));
if (IsSet1 == nullptr)
return true;
- auto *IsSet2 = cast_or_null<BoolValue>(
- cast<StructValue>(&Val2)->getProperty("is_set"));
+ auto *IsSet2 = cast_or_null<BoolValue>(Val2.getProperty("is_set"));
if (IsSet2 == nullptr)
return false;
@@ -365,18 +362,16 @@ class SpecialBoolAnalysis
Decl->getName() != "SpecialBool")
return true;
- auto *IsSet1 = cast_or_null<BoolValue>(
- cast<StructValue>(&Val1)->getProperty("is_set"));
+ auto *IsSet1 = cast_or_null<BoolValue>(Val1.getProperty("is_set"));
if (IsSet1 == nullptr)
return true;
- auto *IsSet2 = cast_or_null<BoolValue>(
- cast<StructValue>(&Val2)->getProperty("is_set"));
+ auto *IsSet2 = cast_or_null<BoolValue>(Val2.getProperty("is_set"));
if (IsSet2 == nullptr)
return true;
auto &IsSet = MergedEnv.makeAtomicBoolValue();
- cast<StructValue>(&MergedVal)->setProperty("is_set", IsSet);
+ MergedVal.setProperty("is_set", IsSet);
if (Env1.flowConditionImplies(*IsSet1) &&
Env2.flowConditionImplies(*IsSet2))
MergedEnv.addToFlowCondition(IsSet);
@@ -426,32 +421,31 @@ TEST_F(JoinFlowConditionsTest, JoinDistinctButProvablyEquivalentValues) {
/*[[p4]]*/
}
)";
- runDataflow(Code,
- [](llvm::ArrayRef<
- std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
- Results,
- ASTContext &ASTCtx) {
- ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
- const Environment &Env1 = Results[3].second.Env;
- const Environment &Env2 = Results[2].second.Env;
- const Environment &Env3 = Results[1].second.Env;
- const Environment &Env4 = Results[0].second.Env;
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
+ Pair("p2", _), Pair("p1", _)));
+ const Environment &Env1 = Results[3].second.Env;
+ const Environment &Env2 = Results[2].second.Env;
+ const Environment &Env3 = Results[1].second.Env;
+ const Environment &Env4 = Results[0].second.Env;
- const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
- ASSERT_THAT(FooDecl, NotNull());
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
- auto GetFooValue = [FooDecl](const Environment &Env) {
- return cast<BoolValue>(
- cast<StructValue>(Env.getValue(*FooDecl, SkipPast::None))
- ->getProperty("is_set"));
- };
+ auto GetFooValue = [FooDecl](const Environment &Env) {
+ return cast<BoolValue>(
+ Env.getValue(*FooDecl, SkipPast::None)->getProperty("is_set"));
+ };
- EXPECT_FALSE(Env1.flowConditionImplies(*GetFooValue(Env1)));
- EXPECT_TRUE(Env2.flowConditionImplies(*GetFooValue(Env2)));
- EXPECT_TRUE(Env3.flowConditionImplies(*GetFooValue(Env3)));
- EXPECT_TRUE(Env4.flowConditionImplies(*GetFooValue(Env3)));
- });
+ EXPECT_FALSE(Env1.flowConditionImplies(*GetFooValue(Env1)));
+ EXPECT_TRUE(Env2.flowConditionImplies(*GetFooValue(Env2)));
+ EXPECT_TRUE(Env3.flowConditionImplies(*GetFooValue(Env3)));
+ EXPECT_TRUE(Env4.flowConditionImplies(*GetFooValue(Env3)));
+ });
}
class OptionalIntAnalysis
@@ -470,7 +464,7 @@ class OptionalIntAnalysis
if (const auto *E = selectFirst<CXXConstructExpr>(
"call", match(cxxConstructExpr(HasOptionalIntType).bind("call"), *S,
getASTContext()))) {
- auto &ConstructorVal = *cast<StructValue>(Env.createValue(E->getType()));
+ auto &ConstructorVal = *Env.createValue(E->getType());
ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(false));
Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
} else if (const auto *E = selectFirst<CXXOperatorCallExpr>(
@@ -487,8 +481,7 @@ class OptionalIntAnalysis
Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
assert(ObjectLoc != nullptr);
- auto &ConstructorVal =
- *cast<StructValue>(Env.createValue(Object->getType()));
+ auto &ConstructorVal = *Env.createValue(Object->getType());
ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(true));
Env.setValue(*ObjectLoc, ConstructorVal);
}
@@ -502,8 +495,7 @@ class OptionalIntAnalysis
Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
return false;
- return cast<StructValue>(&Val1)->getProperty("has_value") ==
- cast<StructValue>(&Val2)->getProperty("has_value");
+ return Val1.getProperty("has_value") == Val2.getProperty("has_value");
}
bool merge(QualType Type, const Value &Val1, const Environment &Env1,
@@ -514,20 +506,18 @@ class OptionalIntAnalysis
Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
return false;
- auto *HasValue1 = cast_or_null<BoolValue>(
- cast<StructValue>(&Val1)->getProperty("has_value"));
+ auto *HasValue1 = cast_or_null<BoolValue>(Val1.getProperty("has_value"));
if (HasValue1 == nullptr)
return false;
- auto *HasValue2 = cast_or_null<BoolValue>(
- cast<StructValue>(&Val2)->getProperty("has_value"));
+ auto *HasValue2 = cast_or_null<BoolValue>(Val2.getProperty("has_value"));
if (HasValue2 == nullptr)
return false;
if (HasValue1 == HasValue2)
- cast<StructValue>(&MergedVal)->setProperty("has_value", *HasValue1);
+ MergedVal.setProperty("has_value", *HasValue1);
else
- cast<StructValue>(&MergedVal)->setProperty("has_value", HasValueTop);
+ MergedVal.setProperty("has_value", HasValueTop);
return true;
}
@@ -598,7 +588,7 @@ TEST_F(WideningTest, JoinDistinctValuesWithDistinctProperties) {
ASSERT_THAT(FooDecl, NotNull());
auto GetFooValue = [FooDecl](const Environment &Env) {
- return cast<StructValue>(Env.getValue(*FooDecl, SkipPast::None));
+ return Env.getValue(*FooDecl, SkipPast::None);
};
EXPECT_EQ(GetFooValue(Env1)->getProperty("has_value"),
@@ -627,34 +617,34 @@ TEST_F(WideningTest, JoinDistinctValuesWithSameProperties) {
/*[[p4]]*/
}
)";
- runDataflow(
- Code, [](llvm::ArrayRef<
- std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
- Results,
- ASTContext &ASTCtx) {
- ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
- const Environment &Env1 = Results[3].second.Env;
- const Environment &Env2 = Results[2].second.Env;
- const Environment &Env3 = Results[1].second.Env;
- const Environment &Env4 = Results[0].second.Env;
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
+ Pair("p2", _), Pair("p1", _)));
+ const Environment &Env1 = Results[3].second.Env;
+ const Environment &Env2 = Results[2].second.Env;
+ const Environment &Env3 = Results[1].second.Env;
+ const Environment &Env4 = Results[0].second.Env;
- const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
- ASSERT_THAT(FooDecl, NotNull());
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
- auto GetFooValue = [FooDecl](const Environment &Env) {
- return cast<StructValue>(Env.getValue(*FooDecl, SkipPast::None));
- };
+ auto GetFooValue = [FooDecl](const Environment &Env) {
+ return Env.getValue(*FooDecl, SkipPast::None);
+ };
- EXPECT_EQ(GetFooValue(Env1)->getProperty("has_value"),
- &Env1.getBoolLiteralValue(false));
- EXPECT_EQ(GetFooValue(Env2)->getProperty("has_value"),
- &Env2.getBoolLiteralValue(true));
- EXPECT_EQ(GetFooValue(Env3)->getProperty("has_value"),
- &Env3.getBoolLiteralValue(true));
- EXPECT_EQ(GetFooValue(Env4)->getProperty("has_value"),
- &Env4.getBoolLiteralValue(true));
- });
+ EXPECT_EQ(GetFooValue(Env1)->getProperty("has_value"),
+ &Env1.getBoolLiteralValue(false));
+ EXPECT_EQ(GetFooValue(Env2)->getProperty("has_value"),
+ &Env2.getBoolLiteralValue(true));
+ EXPECT_EQ(GetFooValue(Env3)->getProperty("has_value"),
+ &Env3.getBoolLiteralValue(true));
+ EXPECT_EQ(GetFooValue(Env4)->getProperty("has_value"),
+ &Env4.getBoolLiteralValue(true));
+ });
}
TEST_F(WideningTest, DistinctPointersToTheSameLocationAreEquivalent) {
@@ -715,8 +705,7 @@ TEST_F(WideningTest, DistinctValuesWithSamePropertiesAreEquivalent) {
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
ASSERT_THAT(FooDecl, NotNull());
- const auto *FooVal =
- cast<StructValue>(Env.getValue(*FooDecl, SkipPast::None));
+ const auto *FooVal = Env.getValue(*FooDecl, SkipPast::None);
EXPECT_EQ(FooVal->getProperty("has_value"),
&Env.getBoolLiteralValue(true));
});
More information about the cfe-commits
mailing list