[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
2022-08-12 16:29:28 +00:00 · 2022-08-12 16:29:28 +00:00 · b3f1a6bf10
parent bc8d966433
commit b3f1a6bf10
6 changed files with 49 additions and 66 deletions
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@ -65,8 +65,9 @@ public:

  /// 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)
--- 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.
--- 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 @@ public:
  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;
  }
 };

--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@ -661,7 +661,7 @@ private:
  // `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);
--- 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:
--- 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