[clang] b3f1a6b - [clang][dataflow] Encode options using llvm::Optional

Sam Estep via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 12 09:29:46 PDT 2022


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


        


More information about the cfe-commits mailing list