Related header and header order in .h files

[Daniel Cheng]

Hello all,

https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes has this to say about include order:

Include headers in the following order: Related header, C system headers, C++ standard library headers, other libraries' headers, your project's headers. 

 

[...] 

 

In dir/foo.cc or dir/foo_test.cc, whose main purpose is to implement or test the stuff in dir2/foo2.h, order your includes as follows: 

 

1. dir2/foo2.h. 

2. A blank line 

3. C system headers (more precisely: headers in angle brackets with the .h extension), e.g., <unistd.h>, <stdlib.h>. 

4. A blank line 

5. C++ standard library headers (without file extension), e.g., <algorithm>, <cstddef>. 

6. A blank line 

7. Other libraries' .h files. 

8. Your project's .h files.

My question is: does 'related header' also apply to .h files? That is, should foo_win.h or foo_impl.h list foo.h first as a related header or not?

I've always assumed that the rule about 'related header' does not apply to .h files; however, the style guide also *doesn't* say that it shouldn't apply to .h files.

While I believe it's more common *not* to apply the related header heuristic to header files (e.g. render_frame_host_impl.h, gl_surface_egl.h), there are also some header files which do include a 'related header' first (e.g. bluetooth_test_win.h).

Thoughts?

Daniel

[Peter Kasting]

On Tue, Aug 31, 2021 at 11:56 AM Daniel Cheng <dch...@chromium.org> wrote:

I've always assumed that the rule about 'related header' does not apply to .h files; however, the style guide also *doesn't* say that it shouldn't apply to .h files.

The guide only explicitly includes "foo.cc or foo_test.cc", not "foo_platform.h".  I was taught to read the examples and explanatory text in the style guide as normative, and under that reading, it wouldn't apply.

PK

[K. Moon]

The example in the style guide is that "dir2/foo2.h" is the related header for "dir/foo.cc" and "dir/foo_test.cc". I think this was changed at some point to make it clear that "related header" does not require an exact match between the names of the .h and .cc files.

The point of the related header rule is to ensure the header includes all necessary headers. (It could also be addressed by having an empty .cc file that just included the header file, and functionally, that's what the style rule accomplishes.)

Given that header files aren't compiled separately from inclusion in a .cc file, I think the proper place to treat such headers as related is in the implementation .cc file, if any; just including them in an .h doesn't make them count as related. If a header is always included indirectly through the same header, there's no value in sorting them in a particular way.

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFDzN8Y1gFJ9M1iWeaqz3B5gTaaeFSZZMSVy1%3DVHiXxckA%40mail.gmail.com.

[Gabriel Charette]

We do commonly apply the related header rule for foo_platform.cc. Not clear to me why not foo_platform.h.

As kmoon said, the idea behind the style rule seems to be to force the translation unit to begin with the most related header (and thus test whether it includes what it uses per C++ parsing rules). Sometimes it adds a bit of readability (e.g. you right away know that foo_win.h is based on foo.h and not just a win-only standalone header).

It gets more fuzzy in foo_impl.h where FooImpl can implement Foo but also BlahObserver, BarNiche, etc. and it's then unclear whether foo.h is really the *most* related header (e.g. render_frame_host_impl.h). But I'd still be fine with having render_frame_host.h up there.

[Daniel Cheng]

On Wed, 1 Sept 2021 at 13:26, Gabriel Charette <g...@chromium.org> wrote:

Le mar. 31 août 2021 15 h 00, 'Peter Kasting' via cxx <c...@chromium.org> a écrit :

On Tue, Aug 31, 2021 at 11:56 AM Daniel Cheng <dch...@chromium.org> wrote:

I've always assumed that the rule about 'related header' does not apply to .h files; however, the style guide also *doesn't* say that it shouldn't apply to .h files.

The guide only explicitly includes "foo.cc or foo_test.cc", not "foo_platform.h".  I was taught to read the examples and explanatory text in the style guide as normative, and under that reading, it wouldn't apply.

We do commonly apply the related header rule for foo_platform.cc. Not clear to me why not foo_platform.h.

Applying the rule to the .cc logically follows from the style guide examples I think. If we have file_win.cc and:

- no file_win.h exists

- file_win.cc is just implementing stuff from file.h

Then I would expect file.h to be the 'related header' for file_win.cc. A simple filename matching heuristic already works relatively well for this.

As kmoon said, the idea behind the style rule seems to be to force the translation unit to begin with the most related header (and thus test whether it includes what it uses per C++ parsing rules). Sometimes it adds a bit of readability (e.g. you right away know that foo_win.h is based on foo.h and not just a win-only standalone header).

It gets more fuzzy in foo_impl.h where FooImpl can implement Foo but also BlahObserver, BarNiche, etc. and it's then unclear whether foo.h is really the *most* related header (e.g. render_frame_host_impl.h). But I'd still be fine with having render_frame_host.h up there.

Peter's comment about foo.cc and foo_test.cc is also the reason I thought that the rule only applied to .cc files.

I feel like it gets more complex with .h files; for example, if a header file declares multiple test fakes (e.g. frame_test_helpers.h from Blink), which header is the related header? Do all of them go at the top? Or would this require that frame_test_helpers.h be split up into smaller units?

Daniel

[Gabriel Charette]

On Wed, Sep 1, 2021 at 4:44 PM Daniel Cheng <dch...@chromium.org> wrote:

On Wed, 1 Sept 2021 at 13:26, Gabriel Charette <g...@chromium.org> wrote:

Le mar. 31 août 2021 15 h 00, 'Peter Kasting' via cxx <c...@chromium.org> a écrit :

On Tue, Aug 31, 2021 at 11:56 AM Daniel Cheng <dch...@chromium.org> wrote:

I've always assumed that the rule about 'related header' does not apply to .h files; however, the style guide also *doesn't* say that it shouldn't apply to .h files.

The guide only explicitly includes "foo.cc or foo_test.cc", not "foo_platform.h".  I was taught to read the examples and explanatory text in the style guide as normative, and under that reading, it wouldn't apply.

We do commonly apply the related header rule for foo_platform.cc. Not clear to me why not foo_platform.h.

Applying the rule to the .cc logically follows from the style guide examples I think. If we have file_win.cc and:

- no file_win.h exists

- file_win.cc is just implementing stuff from file.h

Then I would expect file.h to be the 'related header' for file_win.cc. A simple filename matching heuristic already works relatively well for this.

As kmoon said, the idea behind the style rule seems to be to force the translation unit to begin with the most related header (and thus test whether it includes what it uses per C++ parsing rules). Sometimes it adds a bit of readability (e.g. you right away know that foo_win.h is based on foo.h and not just a win-only standalone header).

It gets more fuzzy in foo_impl.h where FooImpl can implement Foo but also BlahObserver, BarNiche, etc. and it's then unclear whether foo.h is really the *most* related header (e.g. render_frame_host_impl.h). But I'd still be fine with having render_frame_host.h up there.

Peter's comment about foo.cc and foo_test.cc is also the reason I thought that the rule only applied to .cc files.

I feel like it gets more complex with .h files; for example, if a header file declares multiple test fakes (e.g. frame_test_helpers.h from Blink), which header is the related header? Do all of them go at the top? Or would this require that frame_test_helpers.h be split up into smaller units?

I agree that in some cases there are multiple "related headers", similar to my foo_impl.h examples, in those cases I've put all the headers in the shared block (no "related header"). And this is where my readability argument breakdown, it's a small plus when it's possible, but it doesn't work in all cases...

As far as the style guide's argument goes (compilation unit), this is still fine if other .cc's include each .h at their top (i.e. it's the first include somewhere and therefore must IWYU). It also doesn't have to be perfect I guess, anything that improves IWYU in 90% of cases is better than nothing but I'd err towards more (usage of related header) is better than less?