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

[Fujii Hironori]

On Wed, Dec 12, 2018 at 7:07 AM Darin Adler  wrote:

> > On Dec 9, 2018, at 10:34 PM, Fujii Hironori 
> wrote:
> >
> > MSVC has /FI option.
> >
> >   /FI (Name Forced Include File) | Microsoft Docs
> >
> https://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
> >
> https://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.
https://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.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Darin Adler]

> On Dec 9, 2018, at 10:34 PM, Fujii Hironori  wrote:
> 
> MSVC has /FI option.
> 
>   /FI (Name Forced Include File) | Microsoft Docs
>   
> https://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
>   
> https://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
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Don . Olmstead]

There was some work done by aperez to try and enable precompiled headers on all 
platforms at https://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  On Behalf Of Fujii 
Hironori
Sent: Sunday, December 9, 2018 10:35 PM
To: Webkit Development List 
Subject: Re: [webkit-dev] WebCorePrefix.h vs. config.h


On Sun, Dec 9, 2018 at 8:22 AM Darin Adler 
mailto:da...@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 ' option.

  
https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html<https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html>
  
https://clang.llvm.org/docs/ClangCommandLineReference.html<https://clang.llvm.org/docs/ClangCommandLineReference.html>

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

  
https://trac.webkit.org/changeset/163032<https://trac.webkit.org/changeset/163032>

MSVC has /FI option.

  /FI (Name Forced Include File) | Microsoft Docs
  
https://docs.microsoft.com/en-us/cpp/build/reference/fi-name-forced-include-file?view=vs-2017<https://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
  
https://docs.microsoft.com/en-us/cpp/build/reference/yu-use-precompiled-header-file?view=vs-2017<https://docs.microsoft.com/en-us/cpp/build/reference/yu-use-precompiled-header-file?view=vs-2017>

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Fujii Hironori]

On Sun, Dec 9, 2018 at 8:22 AM Darin Adler  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 ' option.

  https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html

  https://clang.llvm.org/docs/ClangCommandLineReference.html


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

  https://trac.webkit.org/changeset/163032


MSVC has /FI option.

  /FI (Name Forced Include File) | Microsoft Docs

https://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

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

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Alex Christensen]

CMake on Mac should not affect the decision here.  I added that as a hack in 
http://trac.webkit.org/r172346 as part of an experimental project that isn’t 
being used by anyone.  If we decide to resume CMake on Mac development and that 
has moved, we will find a better way to solve the same build problem.

> On Dec 8, 2018, at 3:22 PM, Darin Adler  wrote:
> 
> 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.)
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Darin Adler]

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.)
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Darin Adler]

> On Dec 8, 2018, at 2:56 PM, Darin Adler  wrote:
> 
>> On Dec 7, 2018, at 2:44 PM, Alexey Proskuryakov  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
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Darin Adler]

Useful background exists in Wikipedia: 
https://en.wikipedia.org/wiki/Prefix_header 
 and 
https://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___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Darin Adler]

> On Dec 7, 2018, at 2:44 PM, Alexey Proskuryakov  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
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] WebCorePrefix.h vs. config.h

[Alexey Proskuryakov]

Hi,

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?

- Alexey

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev