Bug 107845 – Disable Clang's -Wreturn-type-c-linkage for Source/WebCore/bindings/v8/

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Hans Wennborg]

Disable Clang's -Wreturn-type-c-linkage for Source/WebCore/bindings/v8/

[Hans Wennborg]

Created attachment 184534 [details]
Patch

[Hans Wennborg]

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]

lgtm

[Adam Barth]

Comment on attachment 184534 [details]
Patch

Ok.  Should we fix the code in npruntime.cpp anyway?

[WebKit Review Bot]

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]

Committed r140800: <http://trac.webkit.org/changeset/140800>

[Hans Wennborg]

(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]

(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]

(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.