<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 12, 2016 at 8:22 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank" class="cremed">zturner@google.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">Sounds good. Just to be clear, you plan to delete the code from clang-tidy, then take the code from clang-format and move it to clang-tidy, and have clang-format call clang-tidy (or otherwise share the code somehow so they both use the same implementation)?</div></blockquote><div><br></div><div>No. clang-tidy just calls into clang-format (the library, not the binary) and if clang-format would change the order of #includes, then clang-tidy will display a warning. I already have implemented this. Will try to clean it up and send out a patch.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>I may still try to implement cross-block reordering in clang-tidy because it's the only way to do it in such a way that it just warns you but doesn't apply fixits, or applies them only if it doesn't break the compile, which is what I need at the moment. But since this is just experimental anyway, it probably shouldn't matter much. And if I end up getting something that works reasonably well, we can always move that code over to clang-format if we want to support cross-block reordering there.</div></div></blockquote><div><br></div><div>It's highly unlikely that we'll be able to share code here. The way #includes are detected and handled are very different between clang-format and clang-tidy. If you want to try to implement this for clang-format, by all means. If you are thinking of implementing this for clang-tidy, just take what we have tried before. I can send you pointers to the code (although it has never made it to upstream clang-tidy).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><div class="gmail_quote"><div dir="ltr">On Fri, Aug 12, 2016 at 11:17 AM Daniel Jasper <<a href="mailto:djasper@google.com" target="_blank" class="cremed">djasper@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">I haven't read the patch, but if Alex is ok, so am I.. just wanted to make sure that we don't spend much more time on this, as we are likely going to remove most of the code..</p>
<div class="gmail_extra"><br><div class="gmail_quote">On Aug 12, 2016 6:42 PM, "Zachary Turner" <<a href="mailto:zturner@google.com" target="_blank" class="cremed">zturner@google.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Ahh, I see. Just to be clear, is there an LGTM to get this path in? I know alexfh@ lgtm'ed it, want to make sure you're ok with this too.</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Aug 12, 2016 at 9:40 AM Daniel Jasper <<a href="mailto:djasper@google.com" target="_blank" class="cremed">djasper@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">The check's implementation will be replaced by a simple call to clang tidy. It will remain a check in clang tidy to continue to cater to both use cases.</p>
<div class="gmail_extra"><br><div class="gmail_quote">On Aug 12, 2016 6:19 PM, "Zachary Turner" <<a href="mailto:zturner@google.com" target="_blank" class="cremed">zturner@google.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">That's actually the reason I think it makes more sense in clang tidy. It can be a configuration option, off by default, and since there is more control over whether to apply fixits, and it doesn't apply fixits by default, it would be easier to iterate on the experimental nature of it without messing up code <br><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Aug 12, 2016 at 9:14 AM Alexander Kornienko <<a href="mailto:alexfh@google.com" target="_blank" class="cremed">alexfh@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">alexfh added a comment.<br>
<br>
In <a href="https://reviews.llvm.org/D23434#513839" rel="noreferrer" target="_blank" class="cremed">https://reviews.llvm.org/<wbr>D23434#513839</a>, @djasper wrote:<br>
<br>
> I think we got confused. We once had tried to write an experimental separate check to comply with Google's style guide. If you want to fiddle around with that, contact me, I can send you pointers. But as I mentioned we moved away from that. And I think it makes more sense to re-create the sort-across-blocks functionality in clang-format and not in clang-tidy.<br>
<br>
<br>
Yep, we definitely got confused. That experimental check actually implemented cross-block sorting, but this caused a bunch of issues. Which makes me think that proper implementation of cross-block include sorting is challenging be it in clang-format or clang-tidy. Clang-format probably makes it even more complex, since a higher safety of transformations is expected from it.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D23434" rel="noreferrer" target="_blank" class="cremed">https://reviews.llvm.org/<wbr>D23434</a><br>
<br>
<br>
<br>
</blockquote></div>
</blockquote></div></div>
</blockquote></div>
</blockquote></div></div>
</blockquote></div>
</div></div></blockquote></div><br></div></div>