[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2816979 , @Szelethus wrote: > Sorry for the absence, took my time to catch up with this series! Really > interesting insight into how a new interface is being designed! I need some > time to digest things, but can'

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Sorry for the absence, took my time to catch up with this series! Really interesting insight into how a new interface is being designed! I need some time to digest things, but can't wait to help on the remaining patches. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-11 Thread Valeriy Savchenko via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0cc3100bf8d1: [analyzer] Introduce a new interface for tracking (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Ok this is amazing, no more comments!! Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:210 + ///much. + virtual Result track(KnownS

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-10 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 351098. vsavchenko marked 4 inline comments as done. vsavchenko added a comment. Fix review remarks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-10 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:110 + /// (inlined defensive checks, returned null). + bool EnableNullFPSuppression = true; +}; xazax.hun wrote: > I vaguely remember we wan

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:160-161 + PathSensitiveBugReport &Report; + std::deque ExpressionHandlers; + std::deque StoreHandlers; + Empty deques can be pretty large: > http

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-09 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. > - ... Implementation of isTracked on it's own is fairly simple. And we can > have users of `isTracked` and `ExpressionHandler` at the same time. Maybe, > once we migrate all uses, `ExpressionHandler` can be safely retired. I think this sounds like a plan. More specifica

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-08 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2804421 , @NoQ wrote: > These are not other topics. We're discussing the overall direction into which > this patchset is a large step. I absolutely welcome this step and am > fascinated to see how it turns out but

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-07 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D103605#2802240 , @vsavchenko wrote: > In D103605#2801681 , @NoQ wrote: > >> > > OK, let's make a pause right here. We again start to go into other topics. These are not other topics. W

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-07 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2801681 , @NoQ wrote: > Inlined defensive check suppressions are indeed pretty ugly; this problem > should ideally be solved in a completely different manner. > > That said, I strongly disagree that "when the tracke

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-06 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D103605#2800101 , @vsavchenko wrote: >>> Actually, tracker dies when this happens. But hey, can you provide a use >>> case when the checker really needs to know when this happens? >> >> Inlined defensive check suppressions are a

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:147 +class ExpressionHandler; +class StoreHandler; + vsavchenko wrote: > xazax.hun wrote: > > During path sensitive analysis we already have a

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-04 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2799853 , @NoQ wrote: > In D103605#2798581 , @vsavchenko > wrote: > >> In D103605#2798171 , @NoQ wrote: >> >>> - Trackers are attac

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-04 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:127 +/// into a special block region. +BlockCapture + }; xazax.hun wrote: > There are some other places were we might have effects that are

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-04 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D103605#2798581 , @vsavchenko wrote: > In D103605#2798171 , @NoQ wrote: > >> - Trackers are attached to bug reports. A single bug report may have >> multiple trackers. >> - Why not have

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:110 + /// (inlined defensive checks, returned null). + bool EnableNullFPSuppression = true; +}; I vaguely remember we wanting to put this def

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-04 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D103605#2798171 , @NoQ wrote: > Ok. Oof. Whoa. That's amazing. > > Let me re-iterate to make sure that my understanding of this patchset is > correct. > > **Chapter 1.** //"A is correlated with B (ρ = 0.56), given C, assuming

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-04 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2798171 , @NoQ wrote: > Ok. Oof. Whoa. That's amazing. > > Let me re-iterate to make sure that my understanding of this patchset is > correct. > > **Chapter 1.** //"A is correlated with B (ρ = 0.56), given C, assumi

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Ok. Oof. Whoa. That's amazing. Let me re-iterate to make sure that my understanding of this patchset is correct. **Chapter 1.** //"A is correlated with B (ρ = 0.56), given C, assuming D and under E conditions"// - Two new entities are introduced: trackers and handlers. -

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349650. vsavchenko added a comment. Add forgotten piece of StoreInfo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/include/clang/StaticAnalyzer/Co

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2796425 , @martong wrote: > In D103605#2796141 , @vsavchenko > wrote: > >> In D103605#2796111 , @martong >> wrote: >> >>> Great in

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D103605#2796141 , @vsavchenko wrote: > In D103605#2796111 , @martong wrote: > >> Great initiative! You haven't mention `markInteresting` functions, but I >> think it would be really i

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349548. vsavchenko added a comment. Minor change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugRe

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko marked an inline comment as done. vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:231 + template + void addExpressionHandlerToTheTop(Args &&... ConstructorArgs) { +addExpressionHandlerToThe

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349537. vsavchenko added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/include/clang/StaticAnalyzer/Core/BugRep

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103605#2796111 , @martong wrote: > Great initiative! You haven't mention `markInteresting` functions, but I > think it would be really important to incorporate that functionality here as > well. IMHO, `markInteresting` sh

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Great initiative! You haven't mention `markInteresting` functions, but I think it would be really important to incorporate that functionality here as well. IMHO, `markInteresting` shouldn't have been part of the public API ever, Checkers should have been calling only th

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349529. vsavchenko added a comment. Tweak some parts of the interface Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/include/clang/StaticAnalyzer/C

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 349511. vsavchenko added a comment. Add docstring for StoreHandler Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103605/new/ https://reviews.llvm.org/D103605 Files: clang/include/clang/StaticAnalyzer/Core

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-03 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, manas, RedDocMD. Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision.