diff --git a/clang/include/clang/Analysis/FlowSensitive/Value.h b/clang/include/clang/Analysis/FlowSensitive/Value.h --- a/clang/include/clang/Analysis/FlowSensitive/Value.h +++ b/clang/include/clang/Analysis/FlowSensitive/Value.h @@ -26,6 +26,9 @@ 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 @@ 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 Properties; }; /// Models a boolean. @@ -215,22 +232,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 Children; - llvm::StringMap Properties; }; } // namespace dataflow diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -178,9 +178,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(Val)) { +/// optional value `OptionalVal`. Returns null if `OptionalVal` is null. +BoolValue *getHasValue(Value *OptionalVal) { + if (OptionalVal) { return cast(OptionalVal->getProperty("has_value")); } return nullptr; @@ -221,8 +221,8 @@ void initializeOptionalReference(const Expr *OptionalExpr, const MatchFinder::MatchResult &, LatticeTransferState &State) { - if (auto *OptionalVal = cast_or_null( - 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 transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr, LatticeTransferState &State) { - if (auto *OptionalVal = cast_or_null( - 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 --- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -312,7 +312,7 @@ if (const auto *E = selectFirst( "call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S, getASTContext()))) { - auto &ConstructorVal = *cast(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( @@ -327,8 +327,7 @@ Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer); assert(ObjectLoc != nullptr); - auto &ConstructorVal = - *cast(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( - cast(&Val1)->getProperty("is_set")); + auto *IsSet1 = cast_or_null(Val1.getProperty("is_set")); if (IsSet1 == nullptr) return true; - auto *IsSet2 = cast_or_null( - cast(&Val2)->getProperty("is_set")); + auto *IsSet2 = cast_or_null(Val2.getProperty("is_set")); if (IsSet2 == nullptr) return false; @@ -365,18 +362,16 @@ Decl->getName() != "SpecialBool") return true; - auto *IsSet1 = cast_or_null( - cast(&Val1)->getProperty("is_set")); + auto *IsSet1 = cast_or_null(Val1.getProperty("is_set")); if (IsSet1 == nullptr) return true; - auto *IsSet2 = cast_or_null( - cast(&Val2)->getProperty("is_set")); + auto *IsSet2 = cast_or_null(Val2.getProperty("is_set")); if (IsSet2 == nullptr) return true; auto &IsSet = MergedEnv.makeAtomicBoolValue(); - cast(&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>> - 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>> + 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( - cast(Env.getValue(*FooDecl, SkipPast::None)) - ->getProperty("is_set")); - }; + auto GetFooValue = [FooDecl](const Environment &Env) { + return cast( + 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( "call", match(cxxConstructExpr(HasOptionalIntType).bind("call"), *S, getASTContext()))) { - auto &ConstructorVal = *cast(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( @@ -487,8 +481,7 @@ Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer); assert(ObjectLoc != nullptr); - auto &ConstructorVal = - *cast(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(&Val1)->getProperty("has_value") == - cast(&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( - cast(&Val1)->getProperty("has_value")); + auto *HasValue1 = cast_or_null(Val1.getProperty("has_value")); if (HasValue1 == nullptr) return false; - auto *HasValue2 = cast_or_null( - cast(&Val2)->getProperty("has_value")); + auto *HasValue2 = cast_or_null(Val2.getProperty("has_value")); if (HasValue2 == nullptr) return false; if (HasValue1 == HasValue2) - cast(&MergedVal)->setProperty("has_value", *HasValue1); + MergedVal.setProperty("has_value", *HasValue1); else - cast(&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(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>> - 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>> + 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(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(Env.getValue(*FooDecl, SkipPast::None)); + const auto *FooVal = Env.getValue(*FooDecl, SkipPast::None); EXPECT_EQ(FooVal->getProperty("has_value"), &Env.getBoolLiteralValue(true)); });