Disable Clang's -Wreturn-type-c-linkage for Source/WebCore/bindings/v8/
Created attachment 184534 [details] Patch
Nico: can you check that I got the gyp stuff right? Abarth: would you like to do the formal review? The warning this suppresses looks like this: ../../third_party/WebKit/Source/WebCore/bindings/v8/npruntime.cpp:359:21: error: 'liveObjectMap' has C-linkage specified, but returns user-defined type 'NPObjectMap &' (aka 'HashMap<NPObject *, NPObject *> &') which is incompatible with C [-Werror,-Wreturn-type-c-linkage] static NPObjectMap& liveObjectMap() ^ ../../third_party/WebKit/Source/WebCore/bindings/v8/npruntime.cpp:367:25: error: 'rootObjectMap' has C-linkage specified, but returns user-defined type 'NPRootObjectMap &' (aka 'HashMap<NPObject *, NPObjectSet *> &') which is incompatible with C [-Werror,-Wreturn-type-c-linkage] static NPRootObjectMap& rootObjectMap() ^ (See also Chromium's http://crbug.com/171432)
lgtm
Comment on attachment 184534 [details] Patch Ok. Should we fix the code in npruntime.cpp anyway?
Comment on attachment 184534 [details] Patch Rejecting attachment 184534 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 184534, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: 5. Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Adam Barth']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Parsed 2 diffs from patch file(s). patch: **** Can't create file /tmp/pp81mPPi : No space left on device patch: **** Can't create file /tmp/ppQInq0l : No space left on device Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Adam Barth']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/16110484
Committed r140800: <http://trac.webkit.org/changeset/140800>
(In reply to comment #4) > (From update of attachment 184534 [details]) > Ok. Should we fix the code in npruntime.cpp anyway? I'm not convinced the warning makes sense here (the functions are static, so can only be called from the current file, i.e. C++ code). We're discussing it on the Clang mailing list. My hope is that we'll fix the warning, otherwise I'll change the code. In any case the suppression is temporary.
(In reply to comment #7) > (In reply to comment #4) > > Ok. Should we fix the code in npruntime.cpp anyway? > > I'm not convinced the warning makes sense here (the functions are static, so can only be called from the current file, i.e. C++ code). We're discussing it on the Clang mailing list. I see no reason to have these internal-linkage functions in npruntime.cpp use C linkage. I would prefer that we fix the problem even if the clang folks decide to eliminate the warning.
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #4) > > > Ok. Should we fix the code in npruntime.cpp anyway? > > > > I'm not convinced the warning makes sense here (the functions are static, so can only be called from the current file, i.e. C++ code). We're discussing it on the Clang mailing list. > > I see no reason to have these internal-linkage functions in npruntime.cpp use C linkage. I would prefer that we fix the problem even if the clang folks decide to eliminate the warning. I'm doing this in bug 108218.