[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

This revision was automatically updated to reflect the committed changes. Closed by commit rL358781: Reapply "[analyzer] Introduce a simplified API for adding custom path notes." (authored by dergachev, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ updated this revision to Diff 195835. NoQ added a comment. Fall back to the previous `std::vector>` approach. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58367/new/ https://reviews.llvm.org/D58367 Files: clang/include/clang/Analysis/ProgramPoint.h clang/include/clang/StaticAn

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ added a comment. In D58367#1462026 , @Szelethus wrote: > Hmmm, interesting. Could there be an issue with `NoteTag` not being trivially > destructible? Yeah, i think you're right. Which means we cannot allocate it in a `BumpPtrAllocator`. Reposito

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

Szelethus added a comment. Hmmm, interesting. Could there be an issue with `NoteTag` not being trivially destructible? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58367/new/ https://reviews.llvm.org/D58367

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ reopened this revision. NoQ added a comment. This revision is now accepted and ready to land. Re-reopen after the Diffusion auto-close. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58367/new/ https://reviews.llvm.org/D58367

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

This revision was automatically updated to reflect the committed changes. Closed by commit rG4d6fb5789fca: Revert "[analyzer] Introduce a simplified API for adding custom path notes." (authored by dergachev.a). Changed prior to commit: https://reviews.llvm.org/D58367?vs=192932&id=192937#toc Re

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ reopened this revision. NoQ marked an inline comment as done. NoQ added a comment. This revision is now accepted and ready to land. Reverted in rC357332 ! http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/30957/steps/check-clang%20asan/log

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

This revision was automatically updated to reflect the committed changes. Closed by commit rC357323: [analyzer] Introduce a simplified API for adding custom path notes. (authored by dergachev, committed by ). Changed prior to commit: https://reviews.llvm.org/D58367?vs=192502&id=192932#toc Repo

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:400 + + friend class Factory; + friend class TagVisitor; xazax.hun wrote: > NoQ wrote: > > xazax.hun wrote: >

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ updated this revision to Diff 192502. NoQ marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58367/new/ https://reviews.llvm.org/D58367 Files: clang/include/clang/Analysis/ProgramPoint.h clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ added a comment. //*un-forgets to actually post comments*// In D58367#1443830 , @Charusso wrote: > Cool! I have found this message-semantic idea useful in Unity where every > `GameObject` could talk with each other in a very generic way > (https://d

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

xazax.hun added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:400 + + friend class Factory; + friend class TagVisitor; NoQ wrote: > xazax.hun wrote: > > Isn't this redundant? > It isn't - i made a privat

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:600 +public: + typedef std::function + Callback; Szelethus wrote: > Prefer using. Thx!~ Comment at: clang/include/clang/StaticAnaly

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ updated this revision to Diff 192501. NoQ marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58367/new/ https://reviews.llvm.org/D58367 Files: clang/include/clang/Analysis/ProgramPoint.h clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

Szelethus accepted this revision. Szelethus added a comment. Amazing work! Thanks! In D58367#1443783 , @NoQ wrote: > Remove memoization for now. Address a few inline comments. I'm mostly done > with this first patch, did i accidentally miss anything? > >

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Cool! I have found this message-semantic idea useful in Unity where every `GameObject` could talk with each other in a very generic way (https://docs.unity3d.com/ScriptReference/GameObject

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ updated this revision to Diff 192372. NoQ marked 6 inline comments as done. NoQ added a comment. Remove memoization for now. Address a few inline comments. I'm mostly done with this first patch, did i accidentally miss anything? In D58367#1402796 , @S

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

baloghadamsoftware added a comment. In D58367#1425922 , @Szelethus wrote: > As I understand it, this solution could be used to entirely get rid of the > current bugreporter visitor structure (at least for checkers), right? The > discussion seems to concl

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ marked an inline comment as done. NoQ added a comment. In D58367#1425922 , @Szelethus wrote: > Would `NoteTag`s be displayed in a dumped exploded graph? That's a great question. Tags themselves are always printed out, together with their description

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

Szelethus added a comment. Would `NoteTag`s be displayed in a dumped exploded graph? In D58367#1405185 , @NoQ wrote: > In D58367#1404722 , @Charusso wrote: > > > So with that, I would out-chain this patch from the

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

xazax.hun accepted this revision. xazax.hun added a comment. LG! It is an interesting idea to use this facility for `trackExpressionValue`. But I would expect such a mechanism to trigger quite often. I wonder if the memory consumption would increase significantly by storing a lambda for almost

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

Charusso added a comment. In D58367#1423451 , @NoQ wrote: > Found a bug! The lambda has to manually make sure that the bug report > actually does have something to do with the checker. I think message-semantic is better than storing some data in two pla

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ updated this revision to Diff 189948. NoQ added a comment. Found a bug! The lambda has to manually make sure that the bug report actually does have something to do with the checker. The attached testcase shows that we definitely need to avoid attaching a MIG checker note to a core.DivideZero

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ updated this revision to Diff 187877. NoQ added a comment. Unblock the unrelated MIGChecker patches by untangling them from this core change. This patch will land after them and now includes an update to the checker that demonstrates the use of (and, well, tests) the new API. Comments were

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

Charusso added a comment. In general I think it would be cool to think all of the problems in the same time as these really depends on the `CoreEngine` and how we operates with `ExplodedNodes`. In D58367#1405185 , @NoQ wrote: > In particular, the implem

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ added a comment. Thx!~ In D58368#1404747 , @Charusso wrote: > This is a cool approach, but it is difficult to use this API in other > checkers. If you do not out-chain D58367 I > would like to see something like tha

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

Charusso requested changes to this revision. Charusso added a comment. This revision now requires changes to proceed. First of all, thanks you for working on this, as I wanted to do the same, but I did not know how to. I did not know also that 15 of the checkers already implements `BugReporterVi

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

baloghadamsoftware added a comment. Thank you for working this. I totally agree with you: whenever the checker spawns a new node in the exploded graph there is no point to leave it unmarked and then revers engineer it. `BugReporterVisitor`s should only care for nodes which are not spawned by th

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ added a reviewer: Charusso. NoQ marked an inline comment as done. NoQ added a subscriber: Charusso. NoQ added a comment. In D58367#1402796 , @Szelethus wrote: > 2. I guess most of the visitors could be changed to this format, do you have > plans to co

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

Szelethus added a comment. I love the idea! It looks way cleaner, tbh how messy visitors are have kept me away from implementing one for my own checker. Couple thoughts: 1. It would be great to have unit tests for this. Side note, I have already managed to make CSA unit tests tun under check-cl

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, jdoerfert, dkrupp, donat.nagy, a.sidorin, szepet. Herald added a project: clang. As i mentioned in D58067#1393674