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) {
----------------
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.
================
Comment at:
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:85
+private:
+ ASTContext &Context;
+ MatchSwitch<DiagnoseState<llvm::DenseSet<SourceLocation>>>
----------------
Do we expect this to modify the ASTContext? Could this be a const ref instead?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127898/new/
https://reviews.llvm.org/D127898
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
