Bug 182312 – JSExports.h should be included as <JavaScriptCore/JSExportMacros.h>

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Don Olmstead]

Currently in some places JSExports.h is being included incorrectly as <runtime/JSExports.h>

[Don Olmstead]

Created attachment 332711 [details]
Patch

[EWS Watchlist]

Attachment 332711 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Don Olmstead]

Created attachment 332712 [details]
Patch

Fix changelog

[Don Olmstead]

Created attachment 332714 [details]
Patch

Leave gtest alone for the moment

[Michael Catanzaro]

Comment on attachment 332714 [details]
Patch

That's really weird; I wonder why it was done this way....

[Don Olmstead]

(In reply to Michael Catanzaro from comment #5)
> Comment on attachment 332714 [details]
> Patch
> 
> That's really weird; I wonder why it was done this way....

Yea no clue but doesn't seem to be needed

[WebKit Commit Bot]

Comment on attachment 332714 [details]
Patch

Clearing flags on attachment: 332714

Committed r227866: <https://trac.webkit.org/changeset/227866>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37049309>

[Fujii Hironori]

(In reply to Michael Catanzaro from comment #5)
> That's really weird; I wonder why it was done this way....

This is not so weird. This is the reason why Source/WebCore/ForwardingHeaders/runtime/JSExportMacros.h exists.

A lot of headers are included in this style. This is the reason why Source/WebCore/ForwardingHeaders exists.

[Michael Catanzaro]

I don't understand... I never noticed Source/WebCore/ForwardingHeaders before... why does it exist...?

[Konstantin Tokarev]

Seems like it's used by Mac and Win only

[Don Olmstead]

(In reply to Michael Catanzaro from comment #11)
> I don't understand... I never noticed Source/WebCore/ForwardingHeaders
> before... why does it exist...?

Definitely need to figure it out so https://bugs.webkit.org/show_bug.cgi?id=182303 can proceed. Actually found this issue while doing that and now there are more compilation errors because of <heap|runtime|etc/> style includes.

[Alex Christensen]

(In reply to Michael Catanzaro from comment #11)
> I don't understand... I never noticed Source/WebCore/ForwardingHeaders
> before... why does it exist...?

I don't think there's a good reason why it exists.

[Don Olmstead]

(In reply to Alex Christensen from comment #14)
> (In reply to Michael Catanzaro from comment #11)
> > I don't understand... I never noticed Source/WebCore/ForwardingHeaders
> > before... why does it exist...?
> 
> I don't think there's a good reason why it exists.

Is the same true for the ForwardingHeaders directory within JavaScriptCore?

I can make patches removing them all.

[Michael Catanzaro]

IMO if nobody knows why they exist, we should remove them.

In doing so, we'll either find the answer in some problem, or everything will be fine and it will remain a mystery.

[Don Olmstead]

(In reply to Michael Catanzaro from comment #16)
> IMO if nobody knows why they exist, we should remove them.
> 
> In doing so, we'll either find the answer in some problem, or everything
> will be fine and it will remain a mystery.

Yea I forked that particular issue into https://bugs.webkit.org/show_bug.cgi?id=182347. I don't think it'll work properly until this https://bugs.webkit.org/show_bug.cgi?id=182303 lands.