<div dir="ltr"><div dir="ltr"><div dir="ltr">On Thu, Dec 13, 2018 at 1:21 PM Arnaud Bienner via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>What I like about having this kind of diagnostics being part of clang itself is that you can turn them into errors using -Werror=, and prevent bugs to even been written, saving you debugging time.</div><div>I'm not sure how static code analysis is done on the projects you are working on, but from my experience, it is usually done as part of the CI (so after you commit your changes): this saves you from having bugs reaching the release stage, but doesn't save you the time spent debugging a silly error that could have been catch by the compiler.</div></div></blockquote><div><br></div><div>FWIW, I think most projects run CI on pull requests, so, the code has to pass CI before it's allowed to hit master. (This is how you keep your master CI green all the time, instead of flashing red-green-red-green.)</div><div><br></div><div>I think any diagnostic in this area is really too fragile to be useful. Consider:</div><div><br></div><div>    struct A {</div><div>        A() { do_foo(); }  // diagnosable and definitely a bug</div><div>        virtual void do_foo() = 0;<br></div><div>    };</div><div><div>    struct B {</div><div>        B() { do_foo(); }  // diagnosable, but possibly correct</div><div>        virtual void do_foo();</div><div>    };</div></div><div>    struct C {</div><div>        C() { foo(); }  // definitely a bug, but not diagnosable</div><div>        void foo() { do_foo(); }  // best practice</div><div>    private:</div><div>        virtual void do_foo() = 0;</div><div>    };</div><div><br></div><div>Clang, GCC, and ICC all diagnose "A". Nobody diagnoses "B". Nobody diagnoses "C", even though it's exactly isomorphic to "A" — and "C" (splitting customization points into a public non-virtual interface and a private virtual implementation) is what we want to teach people to do!</div><div><br></div><div>I think Clang can't <i>remove</i> its diagnostic for "A" because that would be a regression versus GCC and ICC; but I don't think it's worth spending time teaching Clang to catch "B" (with its attendant false positives) given that "C" is the most interesting case in practice, and Clang will never be able to catch "C".</div><div><br></div><div>The static analyzer, though, could definitely catch "C"! I don't know if it does today or not.</div><div><br></div><div>my $.02,</div><div>–Arthur</div></div></div></div></div>