[clang-tools-extra] r286222 - [clang-tidy] clang-analyzer-alpha* checks are not registered, so there's no need to disable them

Devin Coughlin via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 9 10:56:01 PST 2016


+ Anna, Alexander, and Artem.

> On Nov 9, 2016, at 10:50 AM, Devin Coughlin via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
> 
>> On Nov 8, 2016, at 9:44 AM, Malcolm Parsons <malcolm.parsons at gmail.com> wrote:
>> 
>> On 8 November 2016 at 16:59, Alexander Kornienko <alexfh at google.com> wrote:
>>> On Nov 8, 2016 2:11 AM, "Malcolm Parsons" <malcolm.parsons at gmail.com> wrote:
>>>> Oh, I was using clang-analyzer-alpha.cplusplus.VirtualCall.
>>>> 
>>>> Should clang-tidy have an option to enable experimental checks?
>>> 
>>> I'd instead ask Static Analyzer folks if they can graduate this check. 
> 
> We agree that this is a valuable checker and are committed to getting it out of alpha. This check is in alpha because:
> 
> a) The diagnostic experience is not very good. It reports a call path directly in the diagnostic message (for example “Call path: foo <— bar” for a call to foo() in bar()) rather than as a path diagnostic.
> 
> b) The lack of path-sensitive reasoning may result in false positives when a called function uses a member variable flag to track whether initialization is complete and does not call the virtual member function during initialization.
> 
> c) The check issues a warning for both calls to pure virtual functions (which is always an error) and non-pure virtual functions (which is more of a code smell and may be a false positive).
> 
> From our perspective, the diagnostic experience is the primary blocker to moving this checker out of alpha and thus turning it on by default.
> 
> Here is our plan to graduate the checker:
> 
> Step 1) Modify the existing AST-based checker to only report on virtual calls in the constructor/destructor itself. This will result in false negatives but avoid the poor diagnostic experience in a). We’ll move the checker out of alpha, which will turn it on by default for all users, and add an -analyzer-config flag to to recover the old behavior.
> 
> Step 2) Rewrite the checker to be path-sensitive. This will improve the diagnostic experience so that we can turn the inter-procedural case back on and also limit false positives from b).
> 
> I’ll commit to doing Step 1) in the immediate future and Step 2) eventually. Once the checker is on by default we’ll need to assess whether the false positive rate from c) is too high — if so, we’ll need to turn the non-pure-virtual case off by default.
> 
> Devin
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list