Re: The order of #include statements

On 15/10/10 06:09, Alex Rousskov wrote:
> Hello,
>
> We require that #include statements are alphabetized, with certain
> exceptions. The presence of old files with out-of-order #includes makes
> it difficult to enforce this policy because it is often difficult to say
> whether the #includes were already 100% ordered without ordering them
> first and then comparing the results -- a time consuming process.
>
> Also, it is not clear what the alphabetical order is, exactly, when
> files contain non-alphabet characters such as '.' and '_'. The "natural"
> order may differ from what a sort(1) command would produce, for example.
> There is also a question of case-sensitivity.
>
> Finally, it is not clear how conditional #include statements should be
> handled, especially when there are multiple #includes under a single
> #ifdef condition.
>
> I suggest that we automate this instead of doing everything manually. It
> should not be very difficult:
>
> 1. #include "" goes before any #include <>.
>
> 2. do not touch the inner #include <> order. Move <> includes carefully
> as they may have #ifdef protections. It is probably easier and safer to
> move "" includes only. No "" includes should be included conditionally
> (this will require a few code changes).

The current policy (which has not been held to) is to wrap all <>
includes and test for them in configure.in.

This could work both for and against automation.
The pro being that we can shuffle at will outside of compat/ and enforce
the wrapping #if. Along with compat/os/* performing undef of the
wrappers after doing any exception includes.

>
> 3. each src/ file, with a few hard-coded exceptions, must start with
> #include "". If there is no "" to include, include "config.h". Do not
> include "config.h" or "squid.h" if there is another "" include.

Why the exception to not include config.h or squid.h?

>
> 4. "config.h", if any, goes first. "squid.h", if any, goes first or
> second, depending on whether there is "config.h"

squid.h requires config.h internally first. If squid.h is included it
overrides config.h being needed.

The goal with these two was to include config.h first outside of src/
and squid.h first inside of src/.

Currently hampered by the long list of dependencies added to the unit
tests by using squid.h. The earlier suggestion to move squid.h to
squid_old.h and shuffle stuff into a cleaner squid.h for src/ may have
to be taken first.

>
> 5. Other "" includes are sorted using ascending ASCII character value
> order. Small letters are capitalized for sorting purposes.

I don't think we need that capitilization clause. Enforcement and
formatting is done centrally so we don't exactly have issues with
portability on the comparisons. ASCII sorting is not a black-box process
to any of us I hope.

>
> I am sure there will be a few exceptions and the above needs some
> polishing, but I think we can automate this pretty well and add to the
> SourceFormating scripts.
>
> Is there a better way to enforce the #include rules? Or should we drop
> the ordering rules instead?

I'm for keeping them.

It's a bit of a no-op on small include lists. But really matters on the
long ones. The patch conflicts from adding a new include at end of the
list are also mostly gone with this policy.

The exceptions will be in compat/ and the third-party library sources
which Henrik has suggested should all be under lib/.

I have a patch ready to test that

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.8
   Beta testers wanted for 3.2.0.2