[webkit-dev] What's the rationale for not including config.h in any header files?

[Alicia Boya García]

On 07/31/2017 11:16 PM, Maciej Stachowiak wrote:
> 
> 
>> On Jul 31, 2017, at 5:04 PM, Michael Catanzaro <mcatanzaro at igalia.com> wrote:
>>
>> On Mon, Jul 31, 2017 at 9:27 PM, Darin Adler <darin at apple.com> wrote:
>>> I don’t think we should add lots of includes of “config.h”, though. I think we can come up with something better.
>>
>> Like what? The only alternative is to pass defines as preprocessor flags via -D. Our command lines are already hard enough to read as it is. Or, well, we could include config.h in Platform.h, where the ENABLE/USE series of macros are defined.
>>
>> I'm curious as to why including config.h in the header file would break Autotools builds. I don't see why that would be the case. As far as I know, it just happens to be common practice to include config.h only in source files. It's special: you always include it first in source files to avoid extremely confusing bugs that can result from not doing so, and therefore it seems redundant to put it in header files.
>>
>> I wonder how other projects (and other IDEs, e.g. XCode) handle this situation. Anyway, I don't have a strong opinion either way....
> 
> I think "config.h" as the first body include, and never in headers, is mostly just a stylistic choice in projects that use autotools.
> 
> However, here are some specific issues to consider:
> (1) Public API headers (or installed private interfaces) should not include config.h, and should not rely on any macros defined there.
> (2) It's probably ok to include it in non-API headers, at least any that use macros defined by config.h.
> (3) Implementation files may need to still include config.h. They might not include any headers that include config.h, or they might currently but don't want to rely on it. config.h may also define macros that affect the behavior of system headers and therefore should be included before any of them, so it needs to go first.
> (4) Given combo of (2) and (3), some includes of config.h in headers may be redundant. I am not sure if this causes any practical problems though.
> 
>  - Maciej
> 
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 

Regarding (1) and (2), the patch I'm proposing only modifies files that
use either USE() or ENABLED() macros. Since public API headers should
not depend on any compilation flags, they should not use either of these
and therefore they will not be modified by the patch.

I agree with (3). All WebKit .cpp files should still include "config.h".

Regarding (4), there should not be any problems as long as the config.h
files are guarded (either by #ifndef or #pragma once). Any effect on
build times should be minimal as the compilers resolve these guards very
quickly (and so far this has been true in my limited tests).

— Alicia.