Bug 107845

Summary: Disable Clang's -Wreturn-type-c-linkage for Source/WebCore/bindings/v8/
Product: WebKit Reporter: Hans Wennborg <hans>
Component: New BugsAssignee: Hans Wennborg <hans>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, thakis, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch abarth: review+, webkit.review.bot: commit-queue-

Hans Wennborg
Reported 2013-01-24 11:05:14 PST
Disable Clang's -Wreturn-type-c-linkage for Source/WebCore/bindings/v8/
Attachments
Patch (1.77 KB, patch)
2013-01-24 11:08 PST, Hans Wennborg
abarth: review+
webkit.review.bot: commit-queue-
Hans Wennborg
Comment 1 2013-01-24 11:08:33 PST
Hans Wennborg
Comment 2 2013-01-24 11:11:37 PST
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)
Nico Weber
Comment 3 2013-01-24 11:16:22 PST
lgtm
Adam Barth
Comment 4 2013-01-24 21:22:36 PST
Comment on attachment 184534 [details] Patch Ok. Should we fix the code in npruntime.cpp anyway?
WebKit Review Bot
Comment 5 2013-01-24 21:24:48 PST
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
Hans Wennborg
Comment 6 2013-01-25 01:37:28 PST
Hans Wennborg
Comment 7 2013-01-25 01:41:54 PST
(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.
Darin Adler
Comment 8 2013-01-29 09:28:14 PST
(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.
Nico Weber
Comment 9 2013-01-29 12:50:38 PST
(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.
Note You need to log in before you can comment on or make changes to this bug.