[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG58fe7f9683a0: [clang][dataflow] Add API to separate analysis from diagnosis (authored by samestep). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep updated this revision to Diff 441112. samestep added a comment. - Tidy AnalysisData struct and PostVisitStmt param Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 Files:

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep updated this revision to Diff 441084. samestep added a comment. - Merge branch 'main' into diagnose-api - Fix minor nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 Files:

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep marked 9 inline comments as done. samestep added a comment. Fixed all the minor nits and responded to some comments. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:401 + const TypeErasedDataflowAnalysisState ) { +

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

sgatev accepted this revision. sgatev added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:401 + const TypeErasedDataflowAnalysisState ) { +PostVisitStmt(Stmt.getStmt(), State); + });

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

gribozavr2 accepted this revision. gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:78 +public: + UncheckedOptionalAccessDiagnosis( + ASTContext , UncheckedOptionalAccessModelOptions Options =

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

xazax.hun accepted this revision. xazax.hun added a comment. Thanks, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 ___ cfe-commits mailing list

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

ymandel added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h:126 +std::function +PostVisitStmt = nullptr); Please update comment to mention this new param (and that its optional)

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep updated this revision to Diff 440687. samestep added a comment. - Implement Gábor's suggestion - Merge branch 'main' into diagnose-api Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 Files:

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep updated this revision to Diff 440406. samestep added a comment. - Bring back Diagnosis.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 Files: clang/docs/tools/clang-formatted-files.txt

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep updated this revision to Diff 440211. samestep added a comment. - Merge branch 'main' into diagnose-api Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 Files:

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep updated this revision to Diff 439878. samestep added a comment. - Merge branch 'main' into diagnose-api - WIP Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 Files:

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:85 +private: + ASTContext + MatchSwitch>> samestep wrote: > samestep wrote: > > xazax.hun wrote: > > > Do we expect this to modify

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep marked an inline comment as done. samestep added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:85 +private: + ASTContext + MatchSwitch>> samestep wrote: > xazax.hun wrote: > > Do we

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:38-42 +llvm::DenseSet diagnoseCFG( +const ControlFlowContext , +std::vector> , +const Environment , TypeErasedDataflowAnalysis , +Diagnosis> Diagnose) {

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep marked an inline comment as done. samestep added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:38-42 +llvm::DenseSet diagnoseCFG( +const ControlFlowContext , +std::vector> , +const Environment ,

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:38-42 +llvm::DenseSet diagnoseCFG( +const ControlFlowContext , +std::vector> , +const Environment , TypeErasedDataflowAnalysis , +Diagnosis> Diagnose) {

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:38-42 +llvm::DenseSet diagnoseCFG( +const ControlFlowContext , +std::vector> , +const Environment , TypeErasedDataflowAnalysis , +Diagnosis> Diagnose) {

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

gribozavr2 added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:38 +template +llvm::DenseSet diagnoseCFG( +const ControlFlowContext , This function seems pretty general and not necessarily tied to diagnostics. WDYT

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep updated this revision to Diff 439411. samestep added a comment. - Remove unnecessary std::move Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 Files: clang/docs/tools/clang-formatted-files.txt

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:57 + } + return std::move(Diags); +} sgatev wrote: > Better to remove this and rely on NRVO? > https://en.cppreference.com/w/cpp/language/copy_elision Ah OK,

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

sgatev accepted this revision. sgatev added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:57 + } + return std::move(Diags); +} Better to remove this and rely on NRVO?

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep updated this revision to Diff 439372. samestep added a comment. - Merge branch 'main' into diagnose-api - Incorporate Gábor's suggestions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 Files:

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:546 +void diagnoseUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr, +DiagnoseState> ) { I think this should

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:76 +class UncheckedOptionalAccessDiagnosis { +public: samestep wrote: > sgatev wrote: > > samestep wrote: > > > samestep wrote: > > >

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:76 +class UncheckedOptionalAccessDiagnosis { +public: sgatev wrote: > samestep wrote: > > samestep wrote: > > > sgatev wrote: > > > >

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

sgatev added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:76 +class UncheckedOptionalAccessDiagnosis { +public: samestep wrote: > samestep wrote: > > sgatev wrote: > > > Move this to a new

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep marked an inline comment as not done. samestep added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:76 +class UncheckedOptionalAccessDiagnosis { +public: samestep wrote: > sgatev wrote: >

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep updated this revision to Diff 439048. samestep added a comment. Try to fix the broken patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 Files: clang/docs/tools/clang-formatted-files.txt

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep marked 4 inline comments as done. samestep added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:49 +template struct DiagnoseState { + DiagnoseState(DiagsT , const Environment ) + : Diags(Diags), Env(Env) {}

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

samestep updated this revision to Diff 439045. samestep added a comment. Comment Diagnosis.h and add missing headers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127898/new/ https://reviews.llvm.org/D127898 Files:

[PATCH] D127898: [clang][dataflow] Add API to separate analysis from diagnosis

sgatev added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h:26 +template +using Diagnosis = +std::function; Let's add some documentation for `Diagnosis` and `diagnoseCFG`. Comment at: