[clang][dataflow] Refactor ApplyBuiltinTransfer field out into DataflowAnalysisOptions struct

[samestep]

Summary

Depends On D130304

This patch pulls the ApplyBuiltinTransfer from the TypeErasedDataflowAnalysis class into a new DataflowAnalysisOptions struct, to allow us to add additional options later without breaking existing code.

[xazax.hun]

clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h

38

I think it might be better to have a default value.

[gribozavr2]

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h

66

Will you remove this bool overload in a later patch?

clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h

38

+1, uninitialized data is tricky to handle -- especially if we add more options in future.

[samestep]

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h

66

I didn't have any plans to, but I'm fine with removing it in a later patch if people want to.

clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h

38

Makes sense, will do!

[samestep]

Give a default value to ApplyBuiltinTransfer

[sgatev]

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h

66

I think it makes sense to remove it. Can you already document that it's deprecated in this patch?

[samestep]

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h

66

Sure, I can document that; what's the best way to do so? Is there a special way to say something is deprecated in a /// comment?

[samestep]

Mark bool constructors as deprecated

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h

 ///     the object is equal to the argument.
 template <typename Derived, typename LatticeT>
 class DataflowAnalysis : public TypeErasedDataflowAnalysis {
 public:
   /// Bounded join-semilattice that is used in the analysis.
   using Lattice = LatticeT;
 
   explicit DataflowAnalysis(ASTContext &Context) : Context(Context) {}
   explicit DataflowAnalysis(ASTContext &Context, bool ApplyBuiltinTransfer)
       : TypeErasedDataflowAnalysis(ApplyBuiltinTransfer), Context(Context) {}
+  explicit DataflowAnalysis(ASTContext &Context,
+                            DataflowAnalysisOptions Options)
+      : TypeErasedDataflowAnalysis(Options), Context(Context) {}
 
   ASTContext &getASTContext() final { return Context; }
 
   TypeErasedLattice typeErasedInitialElement() final {
     return {static_cast<Derived *>(this)->initialElement()};
   }
 
   LatticeJoinEffect joinTypeErased(TypeErasedLattice &E1,

clang/include/clang/Analysis/FlowSensitive/NoopAnalysis.h

public:
   /// `ApplyBuiltinTransfer` controls whether to run the built-in transfer
   /// functions that model memory during the analysis. Their results are not
   /// used by `NoopAnalysis`, but tests that need to inspect the environment
   /// should enable them.
   NoopAnalysis(ASTContext &Context, bool ApplyBuiltinTransfer)
       : DataflowAnalysis<NoopAnalysis, NoopLattice>(Context,
                                                     ApplyBuiltinTransfer) {}
 
+  NoopAnalysis(ASTContext &Context, DataflowAnalysisOptions Options)
+      : DataflowAnalysis<NoopAnalysis, NoopLattice>(Context, Options) {}
+
   static NoopLattice initialElement() { return {}; }
 
   void transfer(const Stmt *S, NoopLattice &E, Environment &Env) {}
 };
 
 } // namespace dataflow
 } // namespace clang
 
 #endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOPANALYSIS_H

clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h

 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
 #include "llvm/ADT/Any.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Error.h"
 
 namespace clang {
 namespace dataflow {
 
+struct DataflowAnalysisOptions {
+  /// Determines whether to apply the built-in transfer functions.
+  // 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;
+};
+
 /// Type-erased lattice element container.
 ///
 /// Requirements:
 ///
 ///  The type of the object stored in the container must be a bounded
 ///  join-semilattice.
 struct TypeErasedLattice {
   llvm::Any Value;
 };
 
 /// Type-erased base class for dataflow analyses built on a single lattice type.
 class TypeErasedDataflowAnalysis : public Environment::ValueModel {
-  /// Determines whether to apply the built-in transfer functions.
-  // 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;
+  DataflowAnalysisOptions Options;
 
 public:
-  TypeErasedDataflowAnalysis() : ApplyBuiltinTransfer(true) {}
+  TypeErasedDataflowAnalysis()
+      : TypeErasedDataflowAnalysis(/*ApplyBuiltinTransfer=*/true) {}
   TypeErasedDataflowAnalysis(bool ApplyBuiltinTransfer)
-      : ApplyBuiltinTransfer(ApplyBuiltinTransfer) {}
+      : Options({ApplyBuiltinTransfer}) {}
+  TypeErasedDataflowAnalysis(DataflowAnalysisOptions Options)
+      : Options(Options) {}
 
   virtual ~TypeErasedDataflowAnalysis() {}
 
   /// Returns the `ASTContext` that is used by the analysis.
   virtual ASTContext &getASTContext() = 0;
 
   /// Returns a type-erased lattice element that models the initial state of a
   /// basic block.
public:
 
   /// Applies the analysis transfer function for a given statement and
   /// type-erased lattice element.
   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 ApplyBuiltinTransfer; }
+  bool applyBuiltinTransfer() const { return Options.ApplyBuiltinTransfer; }
 };
 
 /// Type-erased model of the program at a given program point.
 struct TypeErasedDataflowAnalysisState {
   /// Type-erased model of a program property.
   TypeErasedLattice Lattice;
 
   /// Model of the state of the program (store and heap).