Re: The order of #include statements

On Tue, 19 Oct 2010 12:32:35 -0600, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> On 10/14/2010 08:41 PM, Amos Jeffries wrote:
>> 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.
>
> We can automate the generation of wrappers for <> and even auto-check
> that configure.in has the right checks. Phase2, perhaps?

Aye. Okay.

>
>>> 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?
>
> It simply wastes a little bit of compilation time. If the above rules
> are followed, config.h or squid.h will always be included at least once
> per translation unit.

Um. Deep. Okay on face value it sits pretty.

What is your opinion of breaking the all-dependencies requirement for .h
in the case of config.h?
I mean requiring that they include all dependencies, except that config.h
should ONLY be included by .c/.cc/.cci files. This would fix the loop
problems with resulting hack we have in compat and the extra precompiler
time waste.

squid.h as it currently exists can't face this exception.

>
>>> 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.
>
> Understood:
> 4. "squid.h" or "config.h", if any, goes first. If both are present,
> include just "squid.h".
>
>> The goal with these two was to include config.h first outside of src/
>> and squid.h first inside of src/.
>
> Wrong goal, IMO. We should have one and only one "first" header
> everywhere (squid.h would be a natural name choice).

Okay.

>
>> 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.
>
> Yes, but that is a lot of work. I think this project can kick in before
> this major reshuffle is finished. The alternative is to stop enforcing
> the alphabetical order of #includes until squid.h is fixed.

Yes. The revised (4) works without any of this.

I'll keep on it as a compat project item separate to the #include
enforcement. Just code the policy and enforcer to treat squid.h and
config.h identically, one will disappear eventually.

>>> 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.
>
>
> No strong opinion here, but capitalizing will place "foo*" and "Foo*"
> close to each other, which may look more "natural".

No strong opinion here either.

Amos
Received on Wed Oct 20 2010 - 01:18:21 MDT