[PATCH] D114234: [clang][dataflow] Add base types for building dataflow analyses

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:48
+/// Type-erased base class for dataflow analyses built on a single lattice type.
+class DataflowAnalysisDynamic {
+public:
----------------
Does the `Dynamic` in the suffix refer to the fact this is using type erasure as opposed to templates? 

I have multiple questions at this point:
* Why do we prefer type erasure over generic programming?
* Do you plan to have non-dynamic counterparts?

Nit: having Dynamic both in the class name and in the method names sounds overly verbose to me.
Nit: please add a comment what dynamic refers to in the name,


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:140
+struct DataflowAnalysisState {
+  DataflowAnalysisState(AnyLatticeElement Lattice, Environment Env)
+      : Lattice(std::move(Lattice)), Env(std::move(Env)) {}
----------------
Do we need a ctor? Or is this a workaround for aggregate initialization not working well with certain library facilities?


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysis.cpp:26
+runDataflowAnalysis(const CFG &Cfg, DataflowAnalysisDynamic &Analysis,
+                    const Environment &InitEnv) {
+  // FIXME: Implement work list-based algorithm to compute the fixed
----------------
Is there a way to query how the CFG was built? Adding an assert to see if `setAlwaysAdd` was set might be useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114234