Bug 162664 - Adopt #pragma once in JavaScriptCore
Summary: Adopt #pragma once in JavaScriptCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords:
Depends on: 162682
Blocks:
  Show dependency treegraph
 
Reported: 2016-09-28 00:22 PDT by Joseph Pecoraro
Modified: 2016-09-28 17:11 PDT (History)
11 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (565.27 KB, patch)
2016-09-28 00:30 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.