Bug 162664

Summary: Adopt #pragma once in JavaScriptCore
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: JavaScriptCoreAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bburg, commit-queue, joepeck, keith_miller, mark.lam, msaboff, ossy, pvollan, ryanhaddad, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 162682    
Bug Blocks:    
Attachments:
Description Flags
[PATCH] Proposed Fix none

Description Joseph Pecoraro 2016-09-28 00:22:01 PDT
Adopt #pragma once in JavaScriptCore.

This is the new style. Updating old files really just removes lines so doesn't really affect history.

It was noted in the webkit-dev thread that we should not touch public API headers:
https://lists.webkit.org/pipermail/webkit-dev/2016-March/028062.html
Comment 1 Joseph Pecoraro 2016-09-28 00:30:24 PDT
Created attachment 290065 [details]
[PATCH] Proposed Fix

Kept my hands busy while I watched the debates
Comment 2 WebKit Commit Bot 2016-09-28 00:33:05 PDT
This patch modifies the WEB_REPLAY inputs generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-input-generator-tests --reset-results`)
Comment 3 WebKit Commit Bot 2016-09-28 01:13:11 PDT
Comment on attachment 290065 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 290065

Committed r206506: <http://trac.webkit.org/changeset/206506>
Comment 4 WebKit Commit Bot 2016-09-28 01:13:15 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Csaba Osztrogonác 2016-09-28 01:59:17 PDT
(In reply to comment #3)
> Comment on attachment 290065 [details]
> [PATCH] Proposed Fix
> 
> Clearing flags on attachment: 290065
> 
> Committed r206506: <http://trac.webkit.org/changeset/206506>

It broke the WinCairo build, see https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/61404 for details. cc-ing port maintainers
Comment 6 Csaba Osztrogonác 2016-09-28 02:37:56 PDT
(In reply to comment #5)
> (In reply to comment #3)
> > Comment on attachment 290065 [details]
> > [PATCH] Proposed Fix
> > 
> > Clearing flags on attachment: 290065
> > 
> > Committed r206506: <http://trac.webkit.org/changeset/206506>
> 
> It broke the WinCairo build, see
> https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/61404
> for details. cc-ing port maintainers

clean build is still in progress: https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/61407

Let's see if it fixes the build issue or not.
Comment 7 Mark Lam 2016-09-28 07:51:28 PDT
I thought we held off on using #pragma once in JavaScriptCore because there's a conflict between the use of regular headers and forwarding headers on Windows.  For example:

#include <runtime/VM.h>
#include <JavaScriptCore/VM.h>

.. would result in a redefinition of the VM class because #pragma once treats them as 2 different headers.  Was this problem already solved?
Comment 8 Saam Barati 2016-09-28 08:32:57 PDT
(In reply to comment #7)
> I thought we held off on using #pragma once in JavaScriptCore because
> there's a conflict between the use of regular headers and forwarding headers
> on Windows.  For example:
> 
> #include <runtime/VM.h>
> #include <JavaScriptCore/VM.h>
> 
> .. would result in a redefinition of the VM class because #pragma once
> treats them as 2 different headers.  Was this problem already solved?

Oh wow I didn't even know that was an issue. I'm not sure if it was solved.
Comment 9 WebKit Commit Bot 2016-09-28 09:20:36 PDT
Re-opened since this is blocked by bug 162682
Comment 10 Per Arne Vollan 2016-09-28 09:39:10 PDT
Committed build fix for Windows in <http://trac.webkit.org/changeset/206521>
Comment 11 Ryan Haddad 2016-09-28 10:11:56 PDT
(In reply to comment #10)
> Committed build fix for Windows in <http://trac.webkit.org/changeset/206521>

I started the rollout process before your build fix was committed. Rolled back in with http://trac.webkit.org/changeset/206525
Comment 12 Joseph Pecoraro 2016-09-28 11:03:20 PDT
Sorry for the mayhem this may have caused. =(

Thanks for the follow-up windows fix.

(In reply to comment #7)
> I thought we held off on using #pragma once in JavaScriptCore because
> there's a conflict between the use of regular headers and forwarding headers
> on Windows.  For example:
> 
> #include <runtime/VM.h>
> #include <JavaScriptCore/VM.h>
> 
> .. would result in a redefinition of the VM class because #pragma once
> treats them as 2 different headers.  Was this problem already solved?

Interesting. I had not know that when I made the changes. Seems Windows did encounter this issue, but if it was only a handful of instances we should be fine.