> On Nov 8, 2016, at 9:44 AM, Malcolm Parsons <malcolm.pars...@gmail.com> wrote:
> 
> On 8 November 2016 at 16:59, Alexander Kornienko <ale...@google.com> wrote:
>> On Nov 8, 2016 2:11 AM, "Malcolm Parsons" <malcolm.pars...@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@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to