llvm dev - Mar 2016 - Any objections to re-sorting #include lines with clang-format/clang-tidy?

The crazy clang-format folks have taught it to sort includes, and
clang-tidy has a nice way to run it across all of LLVM and Clang.

Before I just submit the fixes (and any bug reports to the clang-format
folks if it sorts things weirdly) I wanted to double check that folks would
be ok with this. My understanding is that the sorting should match the LLVM
conventions, but if not, I'll make sure to get that fixed first.

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160311/283043bd/attachment.html>

On Fri, Mar 11, 2016 at 11:14 AM, Chandler Carruth via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
> The crazy clang-format folks have taught it to sort includes, and
clang-tidy
> has a nice way to run it across all of LLVM and Clang.
>
> Before I just submit the fixes (and any bug reports to the clang-format
> folks if it sorts things weirdly) I wanted to double check that folks would
> be ok with this. My understanding is that the sorting should match the LLVM
> conventions, but if not, I'll make sure to get that fixed first.
Should take care of windows headers, part of DIASupport.h:

// atlbase.h has to come before windows.h
#include <atlbase.h>
#include <windows.h>

// DIA headers must come after windows headers.
                                                   #include
<cvconst.h>
#include <dia2.h>

Sorting those would break stuff.

Yes, anything like this would be a bug in the logic and I'll file it rather
than submit it.

On Fri, Mar 11, 2016 at 10:18 AM Ismail Donmez via cfe-dev <
cfe-dev at lists.llvm.org> wrote:
> On Fri, Mar 11, 2016 at 11:14 AM, Chandler Carruth via cfe-dev
> <cfe-dev at lists.llvm.org> wrote:
> > The crazy clang-format folks have taught it to sort includes, and
> clang-tidy
> > has a nice way to run it across all of LLVM and Clang.
> >
> > Before I just submit the fixes (and any bug reports to the
clang-format
> > folks if it sorts things weirdly) I wanted to double check that folks
> would
> > be ok with this. My understanding is that the sorting should match the
> LLVM
> > conventions, but if not, I'll make sure to get that fixed first.
>
> Should take care of windows headers, part of DIASupport.h:
>
> // atlbase.h has to come before windows.h
> #include <atlbase.h>
> #include <windows.h>
>
> // DIA headers must come after windows headers.
>                                                    #include
> <cvconst.h>
> #include <dia2.h>
>
> Sorting those would break stuff.
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160311/5708a28f/attachment.html>

If I remember correctly there are some cases of "weird main header" in
LLVM (e.g. Passes.h). I could never form a strong enough opinion to
just sort them as a normal non-main header. Otherwise go for it :)


On Fri, Mar 11, 2016 at 10:14 AM, Chandler Carruth via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
> The crazy clang-format folks have taught it to sort includes, and
clang-tidy
> has a nice way to run it across all of LLVM and Clang.
>
> Before I just submit the fixes (and any bug reports to the clang-format
> folks if it sorts things weirdly) I wanted to double check that folks would
> be ok with this. My understanding is that the sorting should match the LLVM
> conventions, but if not, I'll make sure to get that fixed first.
>
> -Chandler
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>

+1

On 03/11/2016 01:14 AM, Chandler Carruth via cfe-dev
wrote:
> The crazy clang-format folks have taught it to sort includes, and 
> clang-tidy has a nice way to run it across all of LLVM and Clang.
>
> Before I just submit the fixes (and any bug reports to the 
> clang-format folks if it sorts things weirdly) I wanted to double 
> check that folks would be ok with this. My understanding is that the 
> sorting should match the LLVM conventions, but if not, I'll make sure 
> to get that fixed first.
>
> -Chandler
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160311/09b54d4e/attachment.html>

> On Mar 11, 2016, at 1:14 AM, Chandler Carruth via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> The crazy clang-format folks have taught it to sort includes, and
clang-tidy has a nice way to run it across all of LLVM and Clang.
> 
> Before I just submit the fixes (and any bug reports to the clang-format
folks if it sorts things weirdly) I wanted to double check that folks would be
ok with this. My understanding is that the sorting should match the LLVM
conventions, but if not, I'll make sure to get that fixed first.
Does it do it the “right” way?  I think it is important for a .cpp file to
include its corresponding header first (to ensure it stays self contained).

-Chris

On Sat, Mar 12, 2016 at 2:02 AM Chris Lattner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
>
> > On Mar 11, 2016, at 1:14 AM, Chandler Carruth via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >
> > The crazy clang-format folks have taught it to sort includes, and
> clang-tidy has a nice way to run it across all of LLVM and Clang.
> >
> > Before I just submit the fixes (and any bug reports to the
clang-format
> folks if it sorts things weirdly) I wanted to double check that folks would
> be ok with this. My understanding is that the sorting should match the LLVM
> conventions, but if not, I'll make sure to get that fixed first.
>
> Does it do it the “right” way?  I think it is important for a .cpp file to
> include its corresponding header first (to ensure it stays self contained).
>
I don't know for sure, but I'm completely on the same page. I'm not
going
to radically change the sorting rules at all. Any differences there will
just be bugs against the tool that I'll file.

So I'm not going to submit a non-"right" sort. =] I'll just
file bugs there.

>
> -Chris
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160312/278e17be/attachment.html>

On Fri, Mar 11, 2016 at 5:02 PM, Chris Lattner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
>
> > On Mar 11, 2016, at 1:14 AM, Chandler Carruth via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >
> > The crazy clang-format folks have taught it to sort includes, and
> clang-tidy has a nice way to run it across all of LLVM and Clang.
> >
> > Before I just submit the fixes (and any bug reports to the
clang-format
> folks if it sorts things weirdly) I wanted to double check that folks would
> be ok with this. My understanding is that the sorting should match the LLVM
> conventions, but if not, I'll make sure to get that fixed first.
>
> Does it do it the “right” way?  I think it is important for a .cpp file to
> include its corresponding header first (to ensure it stays self contained).
>
Believe it does, yes. At least for the basic/obvious cases:

foo.cpp containing includes of b.h, foo.h, a.h -> foo.h, a.h, b.h

(also, with modules builds, it may no longer be necessary to include the
primary header first to ensure it remains self contained (but yeah, until
we're all using modules builds, it'll be useful so that non-modular
builds
don't silently break this invariant))

>
> -Chris
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160311/325dad05/attachment.html>