wyt updated this revision to Diff 440304. wyt added a comment. Assert that user does not try to substitute true/false booleans.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128658/new/ https://reviews.llvm.org/D128658 Files: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp Index: clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp =================================================================== --- clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp +++ clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp @@ -16,6 +16,7 @@ using namespace clang; using namespace dataflow; +using testing::_; class DataflowAnalysisContextTest : public ::testing::Test { protected: @@ -276,6 +277,32 @@ Context.getOrCreateConjunction(X, Context.getOrCreateConjunction(Y, Z)))); } +TEST_F(DataflowAnalysisContextTest, SubstituteFlowConditionsTrueUnchanged) { + auto &True = Context.getBoolLiteralValue(true); + auto &Other = Context.createAtomicBoolValue(); + + // FC = True + auto &FC = Context.makeFlowConditionToken(); + Context.addFlowConditionConstraint(FC, True); + + // `True` should never be substituted + EXPECT_DEATH(Context.buildAndSubstituteFlowCondition(FC, {{&True, &Other}}), + _); +} + +TEST_F(DataflowAnalysisContextTest, SubstituteFlowConditionsFalseUnchanged) { + auto &False = Context.getBoolLiteralValue(false); + auto &Other = Context.createAtomicBoolValue(); + + // FC = False + auto &FC = Context.makeFlowConditionToken(); + Context.addFlowConditionConstraint(FC, False); + + // `False` should never be substituted + EXPECT_DEATH(Context.buildAndSubstituteFlowCondition(FC, {{&False, &Other}}), + _); +} + TEST_F(DataflowAnalysisContextTest, SubstituteFlowConditionsAtomicFC) { auto &X = Context.createAtomicBoolValue(); auto &True = Context.getBoolLiteralValue(true); Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp +++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp @@ -174,8 +174,12 @@ llvm::DenseMap<BoolValue *, BoolValue *> &SubstitutionsCache) { auto IT = SubstitutionsCache.find(&Val); if (IT != SubstitutionsCache.end()) { + // Return memoized result of substituting this boolean value. return *IT->second; } + + // Handle substitution on the boolean value (and its subvalues), saving the + // result into `SubstitutionsCache`. BoolValue *Result; switch (Val.getKind()) { case Value::Kind::AtomicBool: { @@ -216,6 +220,10 @@ BoolValue &DataflowAnalysisContext::buildAndSubstituteFlowCondition( AtomicBoolValue &Token, llvm::DenseMap<AtomicBoolValue *, BoolValue *> Substitutions) { + // Do not substitute true/false boolean literals. + assert( + Substitutions.find(&getBoolLiteralValue(true)) == Substitutions.end() && + Substitutions.find(&getBoolLiteralValue(false)) == Substitutions.end()); llvm::DenseMap<BoolValue *, BoolValue *> SubstitutionsCache( Substitutions.begin(), Substitutions.end()); return buildAndSubstituteFlowConditionWithCache(Token, SubstitutionsCache);
Index: clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp =================================================================== --- clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp +++ clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp @@ -16,6 +16,7 @@ using namespace clang; using namespace dataflow; +using testing::_; class DataflowAnalysisContextTest : public ::testing::Test { protected: @@ -276,6 +277,32 @@ Context.getOrCreateConjunction(X, Context.getOrCreateConjunction(Y, Z)))); } +TEST_F(DataflowAnalysisContextTest, SubstituteFlowConditionsTrueUnchanged) { + auto &True = Context.getBoolLiteralValue(true); + auto &Other = Context.createAtomicBoolValue(); + + // FC = True + auto &FC = Context.makeFlowConditionToken(); + Context.addFlowConditionConstraint(FC, True); + + // `True` should never be substituted + EXPECT_DEATH(Context.buildAndSubstituteFlowCondition(FC, {{&True, &Other}}), + _); +} + +TEST_F(DataflowAnalysisContextTest, SubstituteFlowConditionsFalseUnchanged) { + auto &False = Context.getBoolLiteralValue(false); + auto &Other = Context.createAtomicBoolValue(); + + // FC = False + auto &FC = Context.makeFlowConditionToken(); + Context.addFlowConditionConstraint(FC, False); + + // `False` should never be substituted + EXPECT_DEATH(Context.buildAndSubstituteFlowCondition(FC, {{&False, &Other}}), + _); +} + TEST_F(DataflowAnalysisContextTest, SubstituteFlowConditionsAtomicFC) { auto &X = Context.createAtomicBoolValue(); auto &True = Context.getBoolLiteralValue(true); Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp +++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp @@ -174,8 +174,12 @@ llvm::DenseMap<BoolValue *, BoolValue *> &SubstitutionsCache) { auto IT = SubstitutionsCache.find(&Val); if (IT != SubstitutionsCache.end()) { + // Return memoized result of substituting this boolean value. return *IT->second; } + + // Handle substitution on the boolean value (and its subvalues), saving the + // result into `SubstitutionsCache`. BoolValue *Result; switch (Val.getKind()) { case Value::Kind::AtomicBool: { @@ -216,6 +220,10 @@ BoolValue &DataflowAnalysisContext::buildAndSubstituteFlowCondition( AtomicBoolValue &Token, llvm::DenseMap<AtomicBoolValue *, BoolValue *> Substitutions) { + // Do not substitute true/false boolean literals. + assert( + Substitutions.find(&getBoolLiteralValue(true)) == Substitutions.end() && + Substitutions.find(&getBoolLiteralValue(false)) == Substitutions.end()); llvm::DenseMap<BoolValue *, BoolValue *> SubstitutionsCache( Substitutions.begin(), Substitutions.end()); return buildAndSubstituteFlowConditionWithCache(Token, SubstitutionsCache);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits