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

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.

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 :slight_smile:

+1

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

When I brought this up, I think the recommendation was to put these into separate include groups (i.e. add a newline between atlbase.h and windows.h – and the DIA headers are already in a separate block). That seems like a good approach to me.

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.

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