Explictly mark stubs called by JIT as being internal
Created attachment 146321 [details] Patch
This is a bit of a rough patch at this point, but I'd appreciate some early thoughts. Bug 88421 tracks some broader symbol visibility issues in the GTK port, but for the JIT it should be possible to explicitly mark stubs as having local visibility, and it doesn't seem like too much of a burden.
Attachment 146321 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/jit/JITStubs.h:463: cti_register_file_check is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/jit/JITStubs.h:464: cti_vm_lazyLinkCall is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/jit/JITStubs.h:465: cti_vm_lazyLinkConstruct is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/jit/JITStubs.h:466: cti_vm_throw is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 4 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Adding Darin as another potential reviewer :)
Comment on attachment 146321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146321&action=review LGTM other than the one comment. Not R+'ing because I wanna know if that can be simplified. > Source/JavaScriptCore/llint/LLIntSlowPaths.h:82 > +#define LLINT_SLOW_PATH_HIDDEN_DECL(name) \ > + LLINT_SLOW_PATH_DECL(name) WTF_INTERNAL Why rename? Is LLINT_SLOW_PATH_DECL used anywhere after this rename?
Thanks for the review. (In reply to comment #5) > > Source/JavaScriptCore/llint/LLIntSlowPaths.h:82 > > +#define LLINT_SLOW_PATH_HIDDEN_DECL(name) \ > > + LLINT_SLOW_PATH_DECL(name) WTF_INTERNAL > > Why rename? Is LLINT_SLOW_PATH_DECL used anywhere after this rename? The rename is because LLINT_SLOW_PATH_DECL is also used in the definitions, and __attribute__ annotations appear to be only allowed in declarations.
(In reply to comment #6) > Thanks for the review. > > (In reply to comment #5) > > > Source/JavaScriptCore/llint/LLIntSlowPaths.h:82 > > > +#define LLINT_SLOW_PATH_HIDDEN_DECL(name) \ > > > + LLINT_SLOW_PATH_DECL(name) WTF_INTERNAL > > > > Why rename? Is LLINT_SLOW_PATH_DECL used anywhere after this rename? > > The rename is because LLINT_SLOW_PATH_DECL is also used in the definitions, and __attribute__ annotations appear to be only allowed in declarations. Oooohhh yeah, I forgot about that ugliness!
R=me. Land with care and make sure you watch the bots, as will I. ;-) I don't expect problems but then again, spoon-feeding compilers tends to be an error-prone thingy.
Comment on attachment 146321 [details] Patch Deal :-) The commit queue looks pretty free, so let's see what happens...
Comment on attachment 146321 [details] Patch Clearing flags on attachment: 146321 Committed r119857: <http://trac.webkit.org/changeset/119857>
All reviewed patches have been landed. Closing bug.