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

[Sam Estep via Phabricator via cfe-commits]

samestep added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:70
+                                       ? TransferOptions{}
+                                       : llvm::Optional<TransferOptions>()}) {}
 
----------------
sgatev wrote:
> 
I tried that, but I get the following error message:

> Incompatible operand types ('TransferOptions' and 'const NoneType')


================
Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:31
+  /// unsupported.
+  llvm::Optional<ContextSensitiveOptions> ContextSensitiveOpts;
 };
----------------
sgatev wrote:
> Perhaps keep the name `ContextSensitive`? In the context of `TransferOptions` it seems clear what this is. 
I don't have a strong preference either way; does anyone else have an opinion on this?


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


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131779