WebCorePrefix.h vs. config.h

# Alexey Proskuryakov (4 days ago)

Do we still need separate WebCorePrefix.h and config.h? The former has this comment, which I don't think is true any more:

/* This prefix file should contain only:

  • 1) files to precompile for faster builds
  • 2) in one case at least: OS-X-specific performance bug workarounds
  • 3) the special trick to catch us using new or delete without including "config.h"
  • The project should be able to build without this header, although we rarely test that. */

/ Things that need to be defined globally should go into "config.h". /

There are many things that contradict this comment in this file. And even when precompiled header is not in use, we include WebCorePrefix.h from config.h anyway:

// Using CMake with Unix makefiles does not use prefix headers.

#if PLATFORM(MAC) && defined(BUILDING_WITH_CMAKE)

#include "WebCorePrefix.h"

#endif

I'm mostly looking at some HAVE and ENABLE macros that are in these and should be elsewhere, but the confusion between these files bothers me a lot. Should we move everything from config.h to WebCorePrefix.h, and only keep config.h just to include WebCorePrefix for the lone build scenario where that's needed, and to undef new/delete?

Contact us to advertise here
# Darin Adler (3 days ago)

Useful background exists in Wikipedia: en.wikipedia.org/wiki/Prefix_header, en.wikipedia.org/wiki/Prefix_header and en.wikipedia.org/wiki/Precompiled_header, en.wikipedia.org/wiki/Precompiled_header are relevant.

Alexey, perhaps you know all of this already, but here’s how I understand the intention behind having both of these files.

The “WebCorePrefix.h” file is a prefix header. We put things here that we want defined everywhere in the project.

The “config.h” file is a workaround for build systems that don’t support a prefix header. It’s inspired by the “config.h” files used in build systems based on autoconf, and was originally intended to keep the WebKit project compatible with them.

The file is unnecessary for builds systems like Xcode that support prefix headers.

Once we decided to use “config.h” as a “pseudo prefix header” we still decided to have a real prefix header under the Xcode build system, at least so we could precompile it. Ideally, under the Xcode build system, including “config.h” should have no effect at all other than participating in the check to help us ensure we didn’t forget to include it for the benefit of other build systems. We wanted to cleverly devise the contents of the prefix header so it’s easy to catch a mistake where we forget to include “config.h” somewhere; we’d like to get compile errors if we forget in most cases.

Maybe we don’t need both of these any more. Two possibilities occur to me:

  • If we don’t need to support systems without support for prefix headers, we can eliminate “config.h”, which would be nice since we can streamline all our source files by removing the include for it.

  • If we can get fast compilation without precompiling a header, then we don’t need to use a prefix header.

However, if we need support for systems without prefix headers and we need to use a prefix header for precompilation, then I do think we need to keep both of these. Their names or contents could change, but I think would still need two separate headers.

It would be great to “purify” any strange properties that these headers have accumulated. There should not be meaningful content repeated in both places. Ideally, content that needs to be included everywhere would be in a single place, perhaps a third header appropriately included by these two, and the prefix header and “pseudo prefix header” would just contain the tricks used to check for their proper use.

A corollary to this is that we should resist the urge to put platform-specific things into the prefix header just because it happens to be used on those platforms and no others. So perhaps there are Cocoa-specific things in WebCorePrefix.h that should instead be in a common place shared by both.

— Darin

# Darin Adler (3 days ago)

On Dec 7, 2018, at 2:44 PM, Alexey Proskuryakov <ap at webkit.org> wrote:

only keep config.h just to include WebCorePrefix for the lone build scenario where that's needed, and to undef new/delete?

I think the answer likely lies here: What is this lone build scenario where config.h is needed?

— Darin

# Darin Adler (3 days ago)

On Dec 8, 2018, at 2:56 PM, Darin Adler <darin at apple.com> wrote:

On Dec 7, 2018, at 2:44 PM, Alexey Proskuryakov <ap at webkit.org> wrote:

only keep config.h just to include WebCorePrefix for the lone build scenario where that's needed, and to undef new/delete?

I think the answer likely lies here: What is this lone build scenario where config.h is needed?

I guess it’s CMake with Unix makefiles. Question for our CMake on Unix experts: Can we get CMake with Unix makefiles to support prefix headers?

— Darin

# Darin Adler (3 days ago)

OK, here’s my answer after thinking on it a bit:

Best would be to eliminate “config.h”: Change “config.h” into an empty file first, then remove all “config.h” includes, and then remove the file. But to do that, we need to make sure every build system for WebKit supports prefix headers. I don’t know how close to that we are. Maybe close? How can we quickly find out?

Lacking that, I think we can and should change “config.h” so it’s just an include of “WebCorePrefix.h”, or the other way around. I think it would be valuable to keep the feature where we try to catch cases where we forget to include “config.h”, on the platforms that use a prefix header, for the benefit of the platforms that do not. That might mean small complexity remains and one file won’t literally just be a trivial include of the other.

I suppose it’s also important to verify that there is no benefit to precompiling less than all of what “config.h” includes.

— Darin

PS: I don’t think we know that there is only one configuration that needs “config.h”. That second code snippet from your original message, Alexey, was only relevant for platforms that are trying to support macOS without prefix headers, and there could be any number of non-macOS cases. (And that include seems like a relatively recent change done by someone who didn’t fully understand the original scheme.)

# Alex Christensen (3 days ago)

CMake

# Fujii Hironori (2 days ago)

On Sun, Dec 9, 2018 at 8:22 AM Darin Adler <darin at apple.com> wrote:

Best would be to eliminate “config.h”: Change “config.h” into an empty file first, then remove all “config.h” includes, and then remove the file. But to do that, we need to make sure every build system for WebKit supports prefix headers. I don’t know how close to that we are. Maybe close? How can we quickly find out?

GCC and Clang support '-include <file>' option.

gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html, gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html, clang.llvm.org/docs/ClangCommandLineReference.html, clang.llvm.org/docs/ClangCommandLineReference.html

GTK and WPE ports are using it only in WK2 since r163032.

trac.webkit.org/changeset/163032, trac.webkit.org/changeset/163032

MSVC has /FI option.

/FI (Name Forced Include File) | Microsoft Docs

docs.microsoft.com/en-us/cpp/build/reference/fi-name-forced-include-file?view=vs-2017, docs.microsoft.com/en-us/cpp/build/reference/fi-name-forced-include-file?view=vs-2017

Unfortunately, it seems that MSVC's precompiled header needs to be included explicitly.

/Yu (Use Precompiled Header File) | Microsoft Docs

docs.microsoft.com/en-us/cpp/build/reference/yu-use-precompiled-header-file?view=vs-2017, docs.microsoft.com/en-us/cpp/build/reference/yu-use-precompiled-header-file?view=vs-2017

# Don.Olmstead at sony.com (a day ago)

There was some work done by aperez to try and enable precompiled headers on all platforms at bugs.webkit.org/show_bug.cgi?id=139438 which never landed. I always thought of WebCorePrefix.h as a precompiled headers sort of optimization.

Last time I checked though there was definitely an issue with how up to date things were in that file and statement that “the project should be able to build without this header, although we rarely test that.” Is definitely false.

From: webkit-dev <webkit-dev-bounces at lists.webkit.org> On Behalf Of Fujii Hironori

Sent: Sunday, December 9, 2018 10:35 PM To: Webkit Development List <webkit-dev at lists.webkit.org>

Subject: Re: [webkit-dev] WebCorePrefix.h vs. config.h

On Sun, Dec 9, 2018 at 8:22 AM Darin Adler <darin at apple.com<mailto:darin at apple.com>> wrote:

Best would be to eliminate “config.h”: Change “config.h” into an empty file first, then remove all “config.h” includes, and then remove the file. But to do that, we need to make sure every build system for WebKit supports prefix headers. I don’t know how close to that we are. Maybe close? How can we quickly find out?

GCC and Clang support '-include <file>' option.

gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.htmlgcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html, clang.llvm.org/docs/ClangCommandLineReference.htmlclang.llvm.org/docs/ClangCommandLineReference.html

GTK and WPE ports are using it only in WK2 since r163032.

trac.webkit.org/changeset/163032trac.webkit.org/changeset/163032

MSVC has /FI option.

/FI (Name Forced Include File) | Microsoft Docs docs.microsoft.com/en-us/cpp/build/reference/fi-name-forced-include-file?view=vs-2017docs.microsoft.com/en-us/cpp/build/reference/fi-name-forced-include-file?view=vs-2017

Unfortunately, it seems that MSVC's precompiled header needs to be included explicitly.

/Yu (Use Precompiled Header File) | Microsoft Docs docs.microsoft.com/en-us/cpp/build/reference/yu-use-precompiled-header-file?view=vs-2017docs.microsoft.com/en-us/cpp/build/reference/yu-use-precompiled-header-file?view=vs-2017

# Darin Adler (9 hours ago)

On Dec 9, 2018, at 10:34 PM, Fujii Hironori <fujii.hironori at gmail.com> wrote:

MSVC has /FI option.

/FI (Name Forced Include File) | Microsoft Docs docs.microsoft.com/en-us/cpp/build/reference/fi-name-forced-include-file?view=vs-2017

Unfortunately, it seems that MSVC's precompiled header needs to be included explicitly.

/Yu (Use Precompiled Header File) | Microsoft Docs docs.microsoft.com/en-us/cpp/build/reference/yu-use-precompiled-header-file?view=vs-2017

So this seems like the main potential obstacle. We can force an include of a prefix header, or precompile a header, but not both, with the Microsoft compiler.

If we deal with this, then I think we could get rid of “config.h”; every other platform can handle a prefix header.

Did I understand that right?

— Darin

# Fujii Hironori (5 hours ago)

On Wed, Dec 12, 2018 at 7:07 AM Darin Adler <darin at apple.com> wrote:

On Dec 9, 2018, at 10:34 PM, Fujii Hironori <fujii.hironori at gmail.com> wrote:

MSVC has /FI option.

/FI (Name Forced Include File) | Microsoft Docs

docs.microsoft.com/en-us/cpp/build/reference/fi-name-forced-include-file?view=vs-2017

Unfortunately, it seems that MSVC's precompiled header needs to be included explicitly.

/Yu (Use Precompiled Header File) | Microsoft Docs

docs.microsoft.com/en-us/cpp/build/reference/yu-use-precompiled-header-file?view=vs-2017

So this seems like the main potential obstacle. We can force an include of a prefix header, or precompile a header, but not both, with the Microsoft compiler.

It was just my misunderstanding. WebKit Windows ports are already using both /Yu and /FI. trac.webkit.org/browser/webkit/trunk/Source/cmake/WebKitMacros.cmake?rev=229282#L100

It seems that WebKit can stop including config.h in all source fils.

Want more features?

Request early access to our private beta of readable email premium.