http://trac.webkit.org/changeset/101198 introduced JS_INLINE and WTF_INLINE but changing JavaScriptCore/config.h was not sufficient.
Created attachment 116720 [details] Patch
Comment on attachment 116720 [details] Patch It seems like a layering violation for WTF to be providing JSC-specific macros.
(In reply to comment #2) > (From update of attachment 116720 [details]) > It seems like a layering violation for WTF to be providing JSC-specific macros. I agree, though like Hajime I'd like to see us get away from defining the various WTF, JS and WebCore export macros in every library that includes headers from them. Perhaps the best idea is to have each library have its own WTFMacros.h, JSMacros.h, etc. that includes wtf/ExportMacros.h and then defines the library-specific ones like JS_EXPORTDATA / CLASS / _PRIVATE. One thing, though, is that this consolidation of macros into a single header might be easier to do once all ports are using the export macros, so I'm not sure if we should tackle it now or just add the macros to the various config.h files (as was done for the previous macros like JS_EXPORTDATA, etc.) and then have the macro consolidation as a separate bug that we make a blocker of the master bug. Thoughts?
Created attachment 116917 [details] Patch
Kevin, Mark, thanks for your feedback! (In reply to comment #2) > (From update of attachment 116720 [details]) > It seems like a layering violation for WTF to be providing JSC-specific macros. This makes sense. So I extracted JSC specific part to JSExportMacros.h and WebKit prefixed ones to PlatformExportMacros.h. Actually this looks unexpectedly better than original one. I originally thought same as Kevin, but the this patch looks natural for me. Note that the style error is false positive. inclusion order matters there.
Created attachment 116919 [details] Patch
Attachment 116919 [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/config.h:33: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/PlatformExportMacros.h:33: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/config.h:37: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSExportMacros.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 4 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 116919 [details] Patch Attachment 116919 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10683111
Comment on attachment 116919 [details] Patch Attachment 116919 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10682291
Created attachment 116929 [details] Patch
Attachment 116929 [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/WebCore/platform/PlatformExportMacros.h:33: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/config.h:37: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSExportMacros.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 3 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 116929 [details] Patch Attachment 116929 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10685058
Comment on attachment 116929 [details] Patch Attachment 116929 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10679573
Created attachment 117147 [details] Patch
Attachment 117147 [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/WebKit2/config.h:37: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptGlue/config.h:6: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptGlue/config.h:7: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/PlatformExportMacros.h:33: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:74: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:75: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:76: Alphabetical sorting problem. [build/include_order] [4] Tools/DumpRenderTree/chromium/config.h:46: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:55: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:56: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:57: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:58: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:59: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:60: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:61: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/runtime/JSExportMacros.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 16 in 30 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 117147 [details] Patch Attachment 117147 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10687140
Created attachment 117349 [details] Patch
Attachment 117349 [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/WebKit2/config.h:37: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptGlue/config.h:6: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptGlue/config.h:7: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/PlatformExportMacros.h:33: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:74: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:75: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:76: Alphabetical sorting problem. [build/include_order] [4] Tools/DumpRenderTree/chromium/config.h:46: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:55: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:56: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:57: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:58: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:59: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:60: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:61: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/runtime/JSExportMacros.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 16 in 30 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 117349 [details] Patch Attachment 117349 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10704510
Created attachment 117393 [details] Patch
Attachment 117393 [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/WebKit2/config.h:37: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptGlue/config.h:6: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptGlue/config.h:7: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/PlatformExportMacros.h:33: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:74: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:75: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:76: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/qt/ChangeLog:8: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Tools/DumpRenderTree/chromium/config.h:46: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:55: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:56: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:57: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:58: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:59: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:60: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:61: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/runtime/JSExportMacros.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 17 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark, Kevin, could you take a look? I think the patch is finally ready to review: The style error is false positive, bots have become green. This patch is a refactoring essentially. What I did is extract duplicate macro definitions in various config.h files to shared ExportMacros.h/JSExportMacros.h/PlatformExportMacros.h header files.
Comment on attachment 117393 [details] Patch Rejecting attachment 117393 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/10725140
Created attachment 117554 [details] Patch for landing
Comment on attachment 117554 [details] Patch for landing Clearing flags on attachment: 117554 Committed r101751: <http://trac.webkit.org/changeset/101751>
All reviewed patches have been landed. Closing bug.
Reverted r101751 for reason: breaks Windows build Committed r101778: <http://trac.webkit.org/changeset/101778>
Created attachment 118962 [details] Patch
This is the same patch as Hajime's except with the added Windows build fixes. I'll land if it now passes all the EWS bots.
Attachment 118962 [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/WebKit2/config.h:37: Alphabetical sorting problem. [build/include_order] [4] Tools/DumpRenderTree/ForwardingHeaders/runtime/JSExportMacros.h:1: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5] Source/JavaScriptGlue/config.h:6: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptGlue/config.h:7: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/PlatformExportMacros.h:33: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:74: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:75: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:76: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/qt/ChangeLog:10: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Tools/DumpRenderTree/chromium/config.h:46: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:55: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:56: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:57: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:58: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:59: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:60: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:61: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/runtime/JSExportMacros.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 18 in 36 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 118962 [details] Patch Attachment 118962 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10846250
Comment on attachment 118962 [details] Patch Attachment 118962 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/10848274
Comment on attachment 118962 [details] Patch Attachment 118962 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10846251
Created attachment 118965 [details] Patch
Attachment 118965 [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/WebKit2/config.h:37: Alphabetical sorting problem. [build/include_order] [4] Tools/DumpRenderTree/ForwardingHeaders/runtime/JSExportMacros.h:1: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5] Source/JavaScriptGlue/config.h:6: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptGlue/config.h:7: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/PlatformExportMacros.h:33: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:74: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:75: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:76: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/qt/ChangeLog:10: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Tools/DumpRenderTree/chromium/config.h:46: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:55: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:56: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:57: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:58: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:59: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:60: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:61: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/runtime/JSExportMacros.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 18 in 36 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 119234 [details] Patch
Attachment 119234 [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/WebKit2/config.h:37: Alphabetical sorting problem. [build/include_order] [4] Tools/DumpRenderTree/ForwardingHeaders/runtime/JSExportMacros.h:1: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5] Source/JavaScriptGlue/config.h:6: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptGlue/config.h:7: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/PlatformExportMacros.h:33: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:74: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:75: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/mac/WebKitPrefix.h:76: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit/qt/ChangeLog:10: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Tools/DumpRenderTree/chromium/config.h:46: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:55: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:56: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:57: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:58: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:59: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:60: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/wtf/text/StringStatics.cpp:61: DEFINE_GLOBAL is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/runtime/JSExportMacros.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 18 in 36 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 119234 [details] Patch Clearing flags on attachment: 119234 Committed r102849: <http://trac.webkit.org/changeset/102849>
Please make sure that you don’t set the svn:executable property on files unnecessarily. It can cause build failures in some environments.
(In reply to comment #40) > Please make sure that you don’t set the svn:executable property on files unnecessarily. It can cause build failures in some environments. Sorry, it was not intentional, but I will be sure to double-check this in the future. Thanks for fixing it!