Re: [lmi] Includes ordering

[Greg Chicares]

On 2018-04-25 16:03, Vadim Zeitlin wrote:
> On Wed, 25 Apr 2018 11:38:03 -0400 (EDT) Greg Chicares <address@hidden> wrote:
> 
> GC> branch: master
> GC> commit e4f1a40cf81bcb9ca65777cb193e198582229cf3
> GC> Author: Vadim Zeitlin <address@hidden>
> GC> Commit: Gregory W. Chicares <address@hidden>
[...]
> GC>     [Amended by GWC to be acceptable to 'hooks/pre-commit'.]
> 
>  Sorry, I forgot to update my pre-commit hook to check concinnity. I'll do
> it now...

Thanks!

[I scripted this--see 'check_git_setup.sh', which is misnamed:
it's like taking ibuprofen to "check" whether you have a fever.]

> GC>  #include <wx/ctrlsub.h>
> GC>  #include <wx/datectrl.h>
> GC> +#include <wx/persist/bookctrl.h>
> GC>  #include <wx/radiobox.h>
> GC>  #include <wx/spinctrl.h>
[...]
>  I do wonder about this rule however: is it really a good idea to
> alphabetically sort headers containing a different number of path
> components? Perhaps it would be preferable to write the above as
> 
>       #include <wx/checkbox.h>
>       #include <wx/ctrlsub.h>
>       #include <wx/datectrl.h>
>       #include <wx/radiobox.h>
>       #include <wx/spinctrl.h>
>       #include <wx/textctrl.h>
> 
>       #include <wx/persist/bookctrl.h>
> 
> instead?

I took a look last time this issue came up, and IIRC we also have
that style in the repository, so it's also accepted by the hook.

For a large library like wx that has a carefully crafted header
hierarchy that expresses something meaningful, your suggestion
above is very appealing. IIRC, e.g., we might include several
related headers from a subdirectory like 'html' or 'xrc', and
writing their #includes in distinct sections makes sense.

OTOH, for a largely flat hierarchy such as the subset of
boost-1.33.1 that we've used, the case for sorting 'filesystem'
headers into their own group seems weaker.

>  More generally, before I add this rule to vz/Style-guide.md, do I
> understand correctly that the order of header groups is:
> 
> 0. The PCH header in the source files or config.hpp in the headers.
> 1. The header corresponding to the current source file.

The header, or headers...I'm sure there's at least one '.cpp' file
that contains implementation for at least two headers.

> 2. Other lmi headers.
> 3. 3rd party library headers (in which order if more than one used?).

In what order? Alphabetical, of course. It seems like a good idea
to separate headers like these
  <boost/any.hpp>
  <wx/defs.h>
with an empty space--each library should have its own clump.

> 4. Standard library headers.

Here's a GPL section of my private documentation that unfortunately
can't be published in toto due to "intellectual property" concerns:

---------8<--------8<--------8<--------8<--------8<--------8<--------8<-------
Group included headers in the following order, sorted alphabetically
within each group, with one blank line between groups.

1 pch file, for .cpp files only
2 config.hpp, for .hpp files only
3 class header, only for .cpp files that implement a class
4 files included with #include "..." (.rh files last)
5 headers for third-party libraries (use #include <...> here)
6 headers defined in the standard

This unmasks implicit dependencies that may make code appear valid
with a particular compiler when other compilers may properly reject it.
--------->8-------->8-------->8-------->8-------->8-------->8-------->8-------

(The last sentence refers to the order of groupings, and is adapted from
Lakos's aging classic, of which a new edition is due July 21, 2018.)

(4) implicitly means files in lmi's own hierarchy only, regardless of
depth, e.g., 'tools/pete-2.1.1/et_vector.hpp':
  #include "et_vector.hpp"
which is presumably found by a compiler '-I' option.

I don't remember what '.rh' was for--probably "resource headers"
for borland-inprise-embarcadero-idera. Just ignore it.

'test_coding_rules.cpp' has functions to check at least some of
these rules. In check_inclusion_order() it tests alphabeticality.