[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 9 13:37:43 PDT 2019


NoQ added a comment.

In D64274#1600834 <https://reviews.llvm.org/D64274#1600834>, @Szelethus wrote:

> If either checker emits an error, the current implementation would say it originates from `cplusplus.PureVirtualCall`. Could you please create a new `ProgramPointTag` with the correct checker name for `optin.cplusplus.VirtualCall`? You may retrieve that name through `CheckerManager::getCurrentCheckName()`.


Whoops right! I addressed this in D65180 <https://reviews.llvm.org/D65180> because it's annoying to rebase the fix through the broken `IsSink` flag in the `reportBug()` function.

In D64274#1600834 <https://reviews.llvm.org/D64274#1600834>, @Szelethus wrote:

> In D64274#1598226 <https://reviews.llvm.org/D64274#1598226>, @NoQ wrote:
>
> > I'm confused though; the way i was previously understanding your work, when disabling a dependency and then enabling a dependent checker *later* in the run-line, it should re-enable the dependency automatically. Did you consciously decide not to implement it that way?
>
>
> Yes, somewhat. I chose another direction, which is simply not exposing base checkers (which I detailed in D60925 <https://reviews.llvm.org/D60925>) to the users (`-analyzer-checker-help-developer` is a thing now), so they aren't ever enabled if none of their dependent checkers are. This worked wonders, because, interestingly, none of the base checkers were emitting diagnostics.
>
> I find this clearer, if I disable something because it crashes on my code, I don't want to see it again. Though, 9.0.0-final isn't out, and I'm sorry that I didn't make this decision clear enough -- shall I fix it?


Yes, generally i think it's good to have later options override earlier options, because it allows people who run analysis through multiple layers of scripts (eg., buildbots) to override their previous decisions without digging through the whole pile of scripts. This isn't *that* important because the use case is pretty rare, but if you have any immediate ideas on how to fix this i'd be pretty happy :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64274/new/

https://reviews.llvm.org/D64274





More information about the cfe-commits mailing list