[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

This revision was automatically updated to reflect the committed changes. Closed by commit rL369449: [analyzer] Improve VirtualCallChecker and enable parts of it by default. (authored by dergachev, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prio

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp:14-20 const char * const CoreFoundationObjectiveC = "Core Foundation/Objective-C"; const char * const LogicError = "Logic error"; const char * con

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Szelethus accepted this revision. Szelethus added a comment. Thank you, and sorry for dragging you through this! At the very least, we got to learn a lot from it :) Comment at: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:93-96 // This wrapper is used to ensure th

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ updated this revision to Diff 215007. NoQ added a comment. Make the checkers independent and extract modeling into a dependency. This means that you can potentially enable the non-pure-virtual call checker without enabling the pure virtual call checker. This doesn't contradict backwards com

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ updated this revision to Diff 215000. NoQ added a comment. I'll merge D65180 into this patch because it's otherwise too painful to update. Like, D65180 untangles bug types and controls enabledness by initializing bug types,

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D64274#1623644 , @NoQ wrote: > In D64274#1600834 , @Szelethus wrote: > > > In D64274#1598226

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ added a comment. In 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 check

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. 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

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ marked an inline comment as done. NoQ added a comment. In D64274#1586282 , @Szelethus wrote: > //Ackchyually//, it doesn't per se break anything, but will result in > CodeChecker no longer enabling `optin.cplusplus.VirtualCall` :^) Sorry, > oversigh

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ added a comment. In D64274#1584974 , @baloghadamsoftware wrote: > Hmm, I still fail to understand the problem with the current `VirtualCall` > checker. Is it unstable? Does it report many false positives? Yeah, pretty much. It's basically defined t

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Szelethus added a comment. Also, shouldn't we add this to the release notes? In general, it's be around time to sort it out (might do that myself before the new branch). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64274/new/ https://reviews.llvm.org/D64274

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:562 + HelpText<"Check virtual function calls during construction/destruction">, Documentation; Szelethus wrote: > Szelethus wrote: > > `Dependencies<[PureVi

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Szelethus added a comment. In D64274#1574086 , @NoQ wrote: > Hmm, wait, i don't really break backwards compatibility. Fridays... //Ackchyually//, it doesn't per se break anything, but will result in CodeChecker no longer enabling `optin.cplusplus.Virtu

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

baloghadamsoftware added a comment. Hmm, I still fail to understand the problem with the current `VirtualCall` checker. Is it unstable? Does it report many false positives? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64274/new/ https://reviews.llvm.org/D64274 _

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Charusso accepted this revision. Charusso added a comment. The `impure-warning` sounds like some alpha, not-well-defined warning, other than that I like the movement to rethink existing checkers. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64274/new/ https://reviews.llvm.org/D64274

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Szelethus added a comment. Please know that I'm currently out of town, so it'll be a while before I can formally accept. Its on top of my list when I get home though! :^) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64274/new/ https://reviews.llvm.org/D64274 ___

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ updated this revision to Diff 208807. NoQ added a comment. Bring back the option in order to preserve backwards compatibility. Sneak in an implicit conversion from `CheckName` to `StringRef` so that not to repeat myself. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64274/new/ htt

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Szelethus added a comment. In D64274#1574118 , @NoQ wrote: > Mmm, no, not really; it seems that if i introduce a checker dependency, i > also have to put the option onto the base checker, otherwise the checker name > wouldn't match when i do > `getCheck

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ updated this revision to Diff 208473. NoQ added a comment. Fix the crash on enabling both checkers, add a test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64274/new/ https://reviews.llvm.org/D64274 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/lib/Stat

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ added a comment. Mmm, no, not really; it seems that if i introduce a checker dependency, i also have to put the option onto the base checker, otherwise the checker name wouldn't match when i do `getCheckerBooleanOption(getChecker(), "PureOnly")`. Which means that the option name will inevi

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ added a comment. Hmm, wait, i don't really break backwards compatibility. Fridays... Previously we have impure-checking when we enable the optin checker and pureonly-checking when we disable the option. I can easily bring back the option, only for the purposes of backwards compatibility, s

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. I happen to have very recent analyses on a couple projects, I'll throw this in: LLVM+Clang+Clang-tools-extra

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet. Herald added a project: clang. This is slightly controversial, please