On Fri, Aug 12, 2016 at 8:22 PM, Zachary Turner <ztur...@google.com> wrote:
> 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)? > 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. 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. > 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). On Fri, Aug 12, 2016 at 11:17 AM Daniel Jasper <djas...@google.com> wrote: > >> 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.. >> >> On Aug 12, 2016 6:42 PM, "Zachary Turner" <ztur...@google.com> wrote: >> >>> 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. >>> >>> On Fri, Aug 12, 2016 at 9:40 AM Daniel Jasper <djas...@google.com> >>> wrote: >>> >>>> 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. >>>> >>>> On Aug 12, 2016 6:19 PM, "Zachary Turner" <ztur...@google.com> wrote: >>>> >>>>> 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 >>>>> >>>>> >>>>> On Fri, Aug 12, 2016 at 9:14 AM Alexander Kornienko <ale...@google.com> >>>>> wrote: >>>>> >>>>>> alexfh added a comment. >>>>>> >>>>>> In https://reviews.llvm.org/D23434#513839, @djasper wrote: >>>>>> >>>>>> > 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. >>>>>> >>>>>> >>>>>> 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. >>>>>> >>>>>> >>>>>> https://reviews.llvm.org/D23434 >>>>>> >>>>>> >>>>>> >>>>>>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits