[PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

[Zachary Turner via cfe-commits]

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)?

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.

On Fri, Aug 12, 2016 at 11:17 AM Daniel Jasper <djasper at 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" <zturner at 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 <djasper at 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" <zturner at 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 <alexfh at 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
>>>>>
>>>>>
>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160812/8357d293/attachment.html>