wyt updated this revision to Diff 435121.
wyt added a comment.
Remove cast to StructValues since get/setProperty can be used directly on
Values now
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127196/new/
https://reviews.llvm.org/D127196
Files:
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -312,7 +312,7 @@
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 @@
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 @@
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 @@
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 @@
/*[[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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
/*[[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 @@
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));
});
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -167,9 +167,9 @@
}
/// 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;
@@ -210,8 +210,8 @@
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());
}
@@ -220,8 +220,8 @@
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);
Index: clang/include/clang/Analysis/FlowSensitive/Value.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/Value.h
+++ clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -26,6 +26,8 @@
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 +50,22 @@
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 +231,8 @@
/// 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
