<div class="gmail_quote">On Tue Jan 13 2015 at 1:54:43 PM Daniel Jasper <<a href="mailto:djasper@google.com">djasper@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">There are many reasons to not put a lot of checks that clang-tidy has into the compiler:<div>a) Checks might be too expensive. Maybe not the case now for this check, but actually removing the braces safely might need quite a bit of additional computation.</div><div>b) Not interesting to many developers. People can have rather strong and diverse opinions on specific style options. In contrast to many of the other Clang warnings, such style issues might increase readability but aren't likely to be bugs.</div><div>c) Doing the checks on every compile is a waste of time. And I don't just mean compilation time. Sometimes when developing code, commenting out sections, etc. checks can suddenly be triggered. Having to fix them (or suppress specific warnings) is additional unnecessary effort. This e.g. is also what bugs some people about -Wunused-*. All we want to do is ensure that these are executed/fixed before submit.</div><div>d) At least for now, checks are way easier to write concisely in clang-tidy. Doing the same work somewhere in Sema can be quite complex. We probably can implement a better abstraction, but that is a lot of work to be done.</div><div><br></div><div>In short, I think we will want to keep many of these checks in clang-tidy even if there isn't a strong technical reason not to move them into the compiler itself. Nonetheless, we should improve the development workflow integration so that we get all of the benefits that we would get from a compiler warning.</div></div></blockquote><div><br></div><div>It still seems like having the ability to compile clang-tidy checks into clang (optionally) and having to run only clang would be a use-case many people might benefit from...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 13, 2015 at 1:12 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Mon, Jan 12, 2015 at 4:07 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Mon, Jan 12, 2015 at 4:00 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Mon, Jan 12, 2015 at 3:46 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>In <a href="http://reviews.llvm.org/D6927#107453" target="_blank">http://reviews.llvm.org/D6927#107453</a>, @dblaikie wrote:<br>
<br>
> Not your problem, but I'm wondering: If/when/how we'll be able to integrate clang-tidy checks into the compile step for developers. This warning and many others in clang-tidy ought to be cheap enough to run at compile time and as hard errors just like many real clang warnings (the only reason they're not is that they're stylistic in nature and so don't meet that bar for the compiler warnings - not because they aren't cheap, low false positive, etc). It'd be nice not to have these as asynchronous results but as errors during the build.<br>
<br>
<br>
</span>Maybe there's scope for a way to add warnings/errors which are run as a separate AST consumer, rather than integrated into the parsing/lexing code path. That way, you don't pay for them if you don't use them (also you don't pay for them in added complexity within the parsing/lexing code). I think most AST-based clang-tidy checks would fit that model. We also have a warning we added internally that would be very nice to cleanly segregate out of the main code path like that.<br></blockquote></div></div><div><br>Perhaps - I'm not sure if "compiled in, but designed to not add complexity to the main codepaths" would meet the bar of the Powers That Be (Richard Smith, mostly, maybe Doug, etc).<br><br>Does anyone care about the size of the clang binary? (I know people care about the size of LLVM so as to be able to use it as a library in web browsers, etc) If not, it seems pretty harmless to put some AST checkers in that way.</div></div></div></div></blockquote><div><br></div></div></div><div>It might be possible to have them as a separate lib that can optionally be linked in based on a configure-time option.</div></div></div></div></blockquote></span><div><br>Yep, would be a possibility, if necessary/desirable.<br> </div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>I suspect that we may have some warnings already that could be excised from the main codepath and put into here, so the net effect might be to enable a slimmer clang for those wanting to reduce clang binary size.<br></div></div></div></div></blockquote></span><div><br>True.<br><br>It'd be interesting to get a feel for the performance of clang-tidy-esque checks, too. I wonder how much of a perf hit it would be to rephrase some existing warnings in terms of AST matching.<span><font color="#888888"><br><br>- David<br> </font></span></div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><span><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><span><font color="#888888"><br><br>- David<br> </font></span></div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div><br>
<br>
<a href="http://reviews.llvm.org/D6927" target="_blank">http://reviews.llvm.org/D6927</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</div></div></blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></div><br></div>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
</blockquote></div>