<div dir="ltr">Malcolm, does it work for you?</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 9, 2016 at 10:56 AM, Devin Coughlin <span dir="ltr"><<a href="mailto:dcoughlin@apple.com" target="_blank">dcoughlin@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+ Anna, Alexander, and Artem.<br>
<div class="HOEnZb"><div class="h5"><br>
> On Nov 9, 2016, at 10:50 AM, Devin Coughlin via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
><br>
><br>
>> On Nov 8, 2016, at 9:44 AM, Malcolm Parsons <<a href="mailto:malcolm.parsons@gmail.com">malcolm.parsons@gmail.com</a>> wrote:<br>
>><br>
>> On 8 November 2016 at 16:59, Alexander Kornienko <<a href="mailto:alexfh@google.com">alexfh@google.com</a>> wrote:<br>
>>> On Nov 8, 2016 2:11 AM, "Malcolm Parsons" <<a href="mailto:malcolm.parsons@gmail.com">malcolm.parsons@gmail.com</a>> wrote:<br>
>>>> Oh, I was using clang-analyzer-alpha.<wbr>cplusplus.VirtualCall.<br>
>>>><br>
>>>> Should clang-tidy have an option to enable experimental checks?<br>
>>><br>
>>> I'd instead ask Static Analyzer folks if they can graduate this check.<br>
><br>
> We agree that this is a valuable checker and are committed to getting it out of alpha. This check is in alpha because:<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> 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).<br>
><br>
> From our perspective, the diagnostic experience is the primary blocker to moving this checker out of alpha and thus turning it on by default.<br>
><br>
> Here is our plan to graduate the checker:<br>
><br>
> 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.<br>
><br>
> 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).<br>
><br>
> 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.<br>
><br>
> Devin<br>
><br>
</div></div><div class="HOEnZb"><div class="h5">> ______________________________<wbr>_________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
<br>
</div></div></blockquote></div><br></div>