Author: Sam Estep Date: 2022-08-12T16:29:41Z New Revision: b3f1a6bf1080fb67cb1760a924a56d38d51211aa
URL: https://github.com/llvm/llvm-project/commit/b3f1a6bf1080fb67cb1760a924a56d38d51211aa DIFF: https://github.com/llvm/llvm-project/commit/b3f1a6bf1080fb67cb1760a924a56d38d51211aa.diff LOG: [clang][dataflow] Encode options using llvm::Optional This patch restructures `DataflowAnalysisOptions` and `TransferOptions` to use `llvm::Optional`, in preparation for adding more sub-options to the `ContextSensitiveOptions` struct introduced here. Reviewed By: sgatev, xazax.hun Differential Revision: https://reviews.llvm.org/D131779 Added: Modified: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h clang/include/clang/Analysis/FlowSensitive/Transfer.h clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h clang/lib/Analysis/FlowSensitive/Transfer.cpp clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp clang/unittests/Analysis/FlowSensitive/TransferTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h index 99e16f8255446..4c785b21526da 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h @@ -65,8 +65,9 @@ class DataflowAnalysis : public TypeErasedDataflowAnalysis { /// Deprecated. Use the `DataflowAnalysisOptions` constructor instead. explicit DataflowAnalysis(ASTContext &Context, bool ApplyBuiltinTransfer) - : DataflowAnalysis(Context, DataflowAnalysisOptions{ApplyBuiltinTransfer, - TransferOptions{}}) {} + : DataflowAnalysis(Context, {ApplyBuiltinTransfer + ? TransferOptions{} + : llvm::Optional<TransferOptions>()}) {} explicit DataflowAnalysis(ASTContext &Context, DataflowAnalysisOptions Options) diff --git a/clang/include/clang/Analysis/FlowSensitive/Transfer.h b/clang/include/clang/Analysis/FlowSensitive/Transfer.h index 8babc5724d46e..93afcc284e1af 100644 --- a/clang/include/clang/Analysis/FlowSensitive/Transfer.h +++ b/clang/include/clang/Analysis/FlowSensitive/Transfer.h @@ -16,17 +16,19 @@ #include "clang/AST/Stmt.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" +#include "llvm/ADT/Optional.h" namespace clang { namespace dataflow { +struct ContextSensitiveOptions {}; + struct TransferOptions { - /// Determines whether to analyze function bodies when present in the - /// translation unit. Note: this is currently only meant to be used for - /// inlining of specialized model code, not for context-sensitive analysis of - /// arbitrary subject code. In particular, some fundamentals such as recursion - /// are explicitly unsupported. - bool ContextSensitive = false; + /// Options for analyzing function bodies when present in the translation + /// unit, or empty to disable context-sensitive analysis. Note that this is + /// fundamentally limited: some constructs, such as recursion, are explicitly + /// unsupported. + llvm::Optional<ContextSensitiveOptions> ContextSensitiveOpts; }; /// Maps statements to the environments of basic blocks that contain them. diff --git a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h index 3bd1c8e12e9b2..58acda7e6389d 100644 --- a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h +++ b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h @@ -32,14 +32,11 @@ namespace clang { namespace dataflow { struct DataflowAnalysisOptions { - /// Determines whether to apply the built-in transfer functions. + /// Options for the built-in transfer functions, or empty to not apply them. // FIXME: Remove this option once the framework supports composing analyses // (at which point the built-in transfer functions can be simply a standalone // analysis). - bool ApplyBuiltinTransfer = true; - - /// Only has an effect if `ApplyBuiltinTransfer` is true. - TransferOptions BuiltinTransferOptions; + llvm::Optional<TransferOptions> BuiltinTransferOpts = TransferOptions{}; }; /// Type-erased lattice element container. @@ -87,13 +84,11 @@ class TypeErasedDataflowAnalysis : public Environment::ValueModel { virtual void transferTypeErased(const Stmt *, TypeErasedLattice &, Environment &) = 0; - /// Determines whether to apply the built-in transfer functions, which model - /// the heap and stack in the `Environment`. - bool applyBuiltinTransfer() const { return Options.ApplyBuiltinTransfer; } - - /// Returns the options to be passed to the built-in transfer functions. - TransferOptions builtinTransferOptions() const { - return Options.BuiltinTransferOptions; + /// If the built-in transfer functions (which model the heap and stack in the + /// `Environment`) are to be applied, returns the options to be passed to + /// them. Otherwise returns empty. + llvm::Optional<TransferOptions> builtinTransferOptions() const { + return Options.BuiltinTransferOpts; } }; diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 2f05b08fe9bc2..1fe8201706840 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -661,7 +661,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { // `F` of `S`. The type `E` must be either `CallExpr` or `CXXConstructExpr`. template <typename E> void transferInlineCall(const E *S, const FunctionDecl *F) { - if (!Options.ContextSensitive) + if (!Options.ContextSensitiveOpts) return; const ControlFlowContext *CFCtx = Env.getControlFlowContext(F); diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp index fe650200e4e36..dc2ecef771b69 100644 --- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp +++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp @@ -200,7 +200,7 @@ static TypeErasedDataflowAnalysisState computeBlockInputState( } llvm::Optional<TypeErasedDataflowAnalysisState> MaybeState; - bool ApplyBuiltinTransfer = Analysis.applyBuiltinTransfer(); + auto BuiltinTransferOpts = Analysis.builtinTransferOptions(); for (const CFGBlock *Pred : Preds) { // Skip if the `Block` is unreachable or control flow cannot get past it. @@ -215,12 +215,12 @@ static TypeErasedDataflowAnalysisState computeBlockInputState( continue; TypeErasedDataflowAnalysisState PredState = MaybePredState.value(); - if (ApplyBuiltinTransfer) { + if (BuiltinTransferOpts) { if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) { const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates); TerminatorVisitor(StmtToEnv, PredState.Env, blockIndexInPredecessor(*Pred, Block), - Analysis.builtinTransferOptions()) + *BuiltinTransferOpts) .Visit(PredTerminatorStmt); } } @@ -255,9 +255,10 @@ static void transferCFGStmt( const Stmt *S = CfgStmt.getStmt(); assert(S != nullptr); - if (Analysis.applyBuiltinTransfer()) + auto BuiltinTransferOpts = Analysis.builtinTransferOptions(); + if (BuiltinTransferOpts) transfer(StmtToEnvMapImpl(CFCtx, BlockStates), *S, State.Env, - Analysis.builtinTransferOptions()); + *BuiltinTransferOpts); Analysis.transferTypeErased(S, State.Lattice, State.Env); if (HandleTransferredStmt != nullptr) @@ -318,7 +319,7 @@ TypeErasedDataflowAnalysisState transferBlock( State, HandleTransferredStmt); break; case CFGElement::Initializer: - if (Analysis.applyBuiltinTransfer()) + if (Analysis.builtinTransferOptions()) transferCFGInitializer(*Element.getAs<CFGInitializer>(), State); break; default: diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 0e33df3a38008..e8b37706d788b 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -64,7 +64,10 @@ void runDataflow(llvm::StringRef Code, Matcher Match, LangStandard::Kind Std = LangStandard::lang_cxx17, bool ApplyBuiltinTransfer = true, llvm::StringRef TargetFun = "target") { - runDataflow(Code, Match, {ApplyBuiltinTransfer, {}}, Std, TargetFun); + runDataflow(Code, Match, + {ApplyBuiltinTransfer ? TransferOptions{} + : llvm::Optional<TransferOptions>()}, + Std, TargetFun); } TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) { @@ -3896,8 +3899,7 @@ TEST(TransferTest, ContextSensitiveOptionDisabled) { EXPECT_FALSE(Env.flowConditionImplies(FooVal)); EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal))); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/false}}); + {TransferOptions{/*.ContextSensitiveOpts=*/llvm::None}}); } TEST(TransferTest, ContextSensitiveSetTrue) { @@ -3926,8 +3928,7 @@ TEST(TransferTest, ContextSensitiveSetTrue) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(FooVal)); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveSetFalse) { @@ -3956,8 +3957,7 @@ TEST(TransferTest, ContextSensitiveSetFalse) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal))); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveSetBothTrueAndFalse) { @@ -3997,8 +3997,7 @@ TEST(TransferTest, ContextSensitiveSetBothTrueAndFalse) { EXPECT_FALSE(Env.flowConditionImplies(BarVal)); EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal))); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveSetTwoLayers) { @@ -4029,8 +4028,7 @@ TEST(TransferTest, ContextSensitiveSetTwoLayers) { EXPECT_FALSE(Env.flowConditionImplies(FooVal)); EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal))); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveSetMultipleLines) { @@ -4071,8 +4069,7 @@ TEST(TransferTest, ContextSensitiveSetMultipleLines) { EXPECT_FALSE(Env.flowConditionImplies(BarVal)); EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BarVal))); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveSetMultipleBlocks) { @@ -4117,8 +4114,7 @@ TEST(TransferTest, ContextSensitiveSetMultipleBlocks) { EXPECT_TRUE(Env.flowConditionImplies(BazVal)); EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(BazVal))); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveReturnVoid) { @@ -4138,8 +4134,7 @@ TEST(TransferTest, ContextSensitiveReturnVoid) { ASSERT_THAT(Results, ElementsAre(Pair("p", _))); // This just tests that the analysis doesn't crash. }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveReturnTrue) { @@ -4166,8 +4161,7 @@ TEST(TransferTest, ContextSensitiveReturnTrue) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(FooVal)); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveReturnFalse) { @@ -4194,8 +4188,7 @@ TEST(TransferTest, ContextSensitiveReturnFalse) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal))); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveReturnArg) { @@ -4225,8 +4218,7 @@ TEST(TransferTest, ContextSensitiveReturnArg) { *cast<BoolValue>(Env.getValue(*BazDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(BazVal)); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveReturnInt) { @@ -4246,8 +4238,7 @@ TEST(TransferTest, ContextSensitiveReturnInt) { ASSERT_THAT(Results, ElementsAre(Pair("p", _))); // This just tests that the analysis doesn't crash. }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveMethodLiteral) { @@ -4278,8 +4269,7 @@ TEST(TransferTest, ContextSensitiveMethodLiteral) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(FooVal)); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveMethodGetter) { @@ -4313,8 +4303,7 @@ TEST(TransferTest, ContextSensitiveMethodGetter) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(FooVal)); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveMethodSetter) { @@ -4348,8 +4337,7 @@ TEST(TransferTest, ContextSensitiveMethodSetter) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(FooVal)); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveMethodGetterAndSetter) { @@ -4385,8 +4373,7 @@ TEST(TransferTest, ContextSensitiveMethodGetterAndSetter) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(FooVal)); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveConstructorBody) { @@ -4419,8 +4406,7 @@ TEST(TransferTest, ContextSensitiveConstructorBody) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(FooVal)); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveConstructorInitializer) { @@ -4453,8 +4439,7 @@ TEST(TransferTest, ContextSensitiveConstructorInitializer) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(FooVal)); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } TEST(TransferTest, ContextSensitiveConstructorDefault) { @@ -4487,8 +4472,7 @@ TEST(TransferTest, ContextSensitiveConstructorDefault) { *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None)); EXPECT_TRUE(Env.flowConditionImplies(FooVal)); }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); + {TransferOptions{ContextSensitiveOptions{}}}); } } // namespace _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
