xazax.hun added inline comments.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:38-42
+llvm::DenseSet<Diag> diagnoseCFG(
+ const ControlFlowContext &CFCtx,
+ std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>> &BlockStates,
+ const Environment &Env, TypeErasedDataflowAnalysis &Analysis,
+ Diagnosis<Lattice, llvm::DenseSet<Diag>> Diagnose) {
----------------
xazax.hun wrote:
> I feel a bit uneasy about this API. The need of helpers in the tests is a
> signal that this API might not be ergonomic as is. One needs to pass a lot of
> stuff in, and it is really error prone. For example, the user might get
> confused what `Environment` to pass in. Or one might pass the wrong
> `Analysis` if there are multiple similar ones in scope. I wonder if there is
> a way to reduce the ceremony needed to get diagnostics out. How about adding
> an optional argument to `runDataflowAnalysis` function to collect the
> results? I wonder if that would be a bit cleaner. You do not need to change
> the return type of that function, the collector object could take a reference
> to the `llvm::DenseSet<Diag>` that it populates.
Moreover, if we pass the diagnostic function to `runDataflowAnalysis`, the fact
whether we collect diagnostic during the fixed-point iteration or after that in
a separate pass becomes an implementation detail. We can easily change back and
forth without affecting the callers. The current API mandates an implementation
approach and makes it harder to change in the future.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127898/new/
https://reviews.llvm.org/D127898
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
