[PATCH] D131779: [clang][dataflow] Encode options using llvm::Optional

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:70
+                                       ? TransferOptions{}
+                                       : llvm::Optional<TransferOptions>()}) {}
 
----------------



================
Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:31
+  /// unsupported.
+  llvm::Optional<ContextSensitiveOptions> ContextSensitiveOpts;
 };
----------------
Perhaps keep the name `ContextSensitive`? In the context of `TransferOptions` it seems clear what this is. 


================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:322
     case CFGElement::Initializer:
-      if (Analysis.applyBuiltinTransfer())
+      if (Analysis.builtinTransferOptions().hasValue())
         transferCFGInitializer(*Element.getAs<CFGInitializer>(), State);
----------------
For consistency.


================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:69
+              {ApplyBuiltinTransfer ? TransferOptions{}
+                                    : llvm::Optional<TransferOptions>()},
+              Std, TargetFun);
----------------



================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3902
               },
-              {/*.ApplyBuiltinTransfer=*/true,
-               /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/false}});
+              {TransferOptions{llvm::Optional<ContextSensitiveOptions>()}});
 }
----------------
xazax.hun wrote:
> I think `llvm::None` is a more concise way to create an empty optional (akin to `std::nullopt`).



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131779/new/

https://reviews.llvm.org/D131779