Bug 123057 – Some includes in JSC seem to use an incorrect style

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

[Alexey Proskuryakov]

What does #include <heap/Weak.h> even mean inside JSC?

[Alexey Proskuryakov]

Created attachment 214642 [details]
proposed patch

FWIW, some of these were introduced as recently as bug 114220, not sure why. Maybe there was a reason?

Let's see if this at least compiles everywhere...

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSContextRef.cpp', u'Source/JavaScriptCore/API/JSStringRefCF.cpp', u'Source/JavaScriptCore/API/JSValueRef.cpp', u'Source/JavaScriptCore/API/OpaqueJSString.cpp', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/jit/JIT.h', u'Source/JavaScriptCore/parser/SyntaxChecker.h', u'Source/JavaScriptCore/runtime/WeakGCMap.h']" exit_code: 1
Source/JavaScriptCore/API/JSValueRef.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 8 files


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

[Alexey Proskuryakov]

Created attachment 214643 [details]
with include order fix

[Geoffrey Garen]

I believe we need <> style for any header that will be included into a WebCore file.

[Geoffrey Garen]

Comment on attachment 214643 [details]
with include order fix

r=me

[Alexey Proskuryakov]

> I believe we need <> style for any header that will be included into a WebCore file.

Some of these headers are included in WebCore... I'm confused :(

[Geoffrey Garen]

My understanding is that, on Mac, WebCore includes JavaScriptCore headers through the ForwardingHeaders folder, which is treated as a non-local include path. Hence the <> and the folder prefix. Maybe this has changed somehow?

[Alexey Proskuryakov]

Looks like here is what we somehow naturally stabilized on:

- JSC API headers are included from other projects as <JavaScriptCore/Foo.h>. This works on Mac because of frameworks, and it works on other platforms because of Source/JavaScriptCore/ForwardingHeaders/JavaScriptCore.

- Other JSC headers are included from other projects as <runtime/Foo.h>. This works because of forwarding headers in those projects, I think.

- JSC headers themselves include other JSC headers as "Foo.h". I couldn't figure out how this works on Mac - perhaps Xcode builds a header map for all JSC private headers? On other platforms, it works because each project adds all JSC subdirectories to search paths.

I have my reservations about the beauty of this scheme (and I could misunderstand something). But now I think that it's good to land this patch for consistency. Also, it will make it easier for me to fix the build in bug 86914.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r157687>.