<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
span.E-MailFormatvorlage19
        {mso-style-type:personal-compose;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-family:"Calibri",sans-serif;
        mso-fareast-language:EN-US;}
@page WordSection1
        {size:612.0pt 792.0pt;
        margin:70.85pt 70.85pt 2.0cm 70.85pt;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=DE link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>Since no-one picked this up, I’ll just comment although I’m not deeply involved. Maybe this will trigger someone else.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>I personally like the idea in general. While there is a risk of stuff being experimental forever there is also more feedback and more contributions that way compared to those checkers being hidden somewhere privately.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>Also I agree on the stability comments. It doesn’t make sense to add rules so bad that they just crash. The goal should be to collect real feedback, real bug reports, etc.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>I’m also thinking of the const adding checker (Sorry I forgot the name), which had like 0,1% false positives and therefore cannot be declared fully ‘working’ yet (as far as I remember).<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>With that checker being easily available to more people maybe some simpler cases with false positives will emerge which are easier to analyze.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>The other questions I leave to the experts ;-)<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>Regards,<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>Alex<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal><b>Von:</b> cfe-dev <cfe-dev-bounces@lists.llvm.org> <b>Im Auftrag von </b>Whisperity via cfe-dev<br><b>Gesendet:</b> Mittwoch, 25. März 2020 13:32<br><b>An:</b> via cfe-dev <cfe-dev@lists.llvm.org>; aaron@aaronballman.com<br><b>Betreff:</b> [cfe-dev] [clang-tidy] RFC: "Experimental" checkers<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal>Dear List, <o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>This is a generic interest pitch mail about adding the notion of "experimental" checks to Clang-Tidy.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>(A *very* rudimentary patch about the experimental group is here: <a href="http://reviews.llvm.org/D76545" target="_blank">http://reviews.llvm.org/D76545</a>.) <o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>The reason behind this idea is from the discussion with Aaron on a check that I developed and proposing for the suite. Review *of the check* here: <a href="http://reviews.llvm.org/D69560" target="_blank">http://reviews.llvm.org/D69560</a>.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Summary (of the experimental idea):<o:p></o:p></p></div><div><p class=MsoNormal> - Certain rules (such as the one mentioned above) cannot (yet?) be implemented properly, due to various constraints<o:p></o:p></p></div><div><p class=MsoNormal> - However, parts of the rule that *could* be implemented might turn out to be useful for the community<o:p></o:p></p></div><div><p class=MsoNormal> - It's not nice to "sell" a checker named "foo-bar" if it only does "bar" partially — but coming up with a different name is superfluous as we wish to convey the intent that we support "bar" fully later on.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>The Clang Static Analyser people have a similar notion of "alpha.", however, several concerns about modelling that approach for Tidy has been expressed, namely:<o:p></o:p></p></div><div><p class=MsoNormal> - the lifespan of checkers in the alpha. group is too long for Tidy's(?) taste<o:p></o:p></p></div><div><p class=MsoNormal> - alpha checkers are allowed to have crashes, lots of false positives, senseless output, etc.<o:p></o:p></p></div><div><p class=MsoNormal> - it's generally a "you're on your own" situation if you decide to run alpha checkers<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>It's understandable that we wish to be something more stable, but this needs more input from a broader set of people.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>I believe that requiring stability (i.e. no crashes in general, or performance issues) from upstramed experimental checkers is a good idea. The core idea of an experimental checker could be:<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>"Here's a rule, it's a nice rule, here's an automated tool for that, let's see what it does on live projects. This can help us develop further, or maybe even do rounds of revising/extending the rule/best practice itself!"<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>----<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>First things first: main group, or subgroup?<o:p></o:p></p></div><div><p class=MsoNormal>We could organise the situation in two ways:<o:p></o:p></p></div><div><p class=MsoNormal> - Having a top level group "experimental-<originally targeted group>-checker-name"<o:p></o:p></p></div><div><p class=MsoNormal> - Or having a "subgroup" under each main group: "<originally targeted group>-experimental-checker-name"<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>In the latter case, I believe *not* turning on experimental checks unless the user explicitly requests (in some fashion) is necessary.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>There're a few key bureaucracy points that need to be discussed before moving forward:<o:p></o:p></p></div><div><p class=MsoNormal> <o:p></o:p></p></div><div><p class=MsoNormal> 1. What's the accepted quality level for a patch to be merged as an experimental check? Where is the line between full-size checker, vs. an experimental?<o:p></o:p></p></div><div><p class=MsoNormal> 2. What's the life cycle concerns involved by experimental checkers?<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Remember, that checkers merged into upstream will be "sold" as an LLVM feature.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>For 1., I think (and hopefully this is a somewhat neutral point of view argument, even though it's my patch) that the checker I'm proposing (<a href="http://reviews.llvm.org/D69560">http://reviews.llvm.org/D69560</a>) is a good example. There's a well-defined idea, and a large part of the implementation *works* in practice. (And - hopefully - there's a small amount of weird crashes here and there, I've did my best to eliminate what I could.)<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>For 2., I'm with more lack of words. To reiterate a claim from the review of the "new group idea": In ClangSA, there are alpha checkers that had been there for years, but so far never made out of alpha, whereas requiring constant maintenance (at least to the point of making sure an internal API break is patched into them...) <o:p></o:p></p></div><div><p class=MsoNormal>"We don't want 'experimental-' to become a dumping ground for bad quality checks."<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>There should be at least the general idea that we can, in the future, declare the experiment to have failed (for whatever reason, be it lack of interest, change of winds, or technical issues), and eventually deprecate and remove the related check.<o:p></o:p></p></div><div><p class=MsoNormal>The other path, where the experiment is a success is easier, hopefully a simple rename (and some aliasing) can solve it.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div></div></div></body></html>