107845 2013-01-24 11:05:14 -0800 Disable Clang's -Wreturn-type-c-linkage for Source/WebCore/bindings/v8/ 2013-01-29 12:50:38 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 hans hans abarth darin thakis webkit.review.bot oldest_to_newest 815628 0 hans 2013-01-24 11:05:14 -0800 Disable Clang's -Wreturn-type-c-linkage for Source/WebCore/bindings/v8/ 815631 1 184534 hans 2013-01-24 11:08:33 -0800 Created attachment 184534 Patch 815638 2 hans 2013-01-24 11:11:37 -0800 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) 815641 3 thakis 2013-01-24 11:16:22 -0800 lgtm 816174 4 184534 abarth 2013-01-24 21:22:36 -0800 Comment on attachment 184534 Patch Ok. Should we fix the code in npruntime.cpp anyway? 816175 5 184534 webkit.review.bot 2013-01-24 21:24:48 -0800 Comment on attachment 184534 Patch Rejecting attachment 184534 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 816379 6 hans 2013-01-25 01:37:28 -0800 Committed r140800: <http://trac.webkit.org/changeset/140800> 816383 7 hans 2013-01-25 01:41:54 -0800 (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. 819016 8 darin 2013-01-29 09:28:14 -0800 (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. 819231 9 thakis 2013-01-29 12:50:38 -0800 (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. 184534 2013-01-24 11:08:33 -0800 2013-01-24 21:24:48 -0800 Patch bug-107845-20130124190525.patch text/plain 1816 hans U3VidmVyc2lvbiBSZXZpc2lvbjogMTQwNjkyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYjFjOGY5NjljYWMwODVi MTJmMDM4Y2JjMmY0YWFkZDViYzBiMGUxOC4uYzcwZDZjNGVlMWViMGM2ZDEyZDJlMWZlMmZlNTlj YmE4MmRjN2MxMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDEzLTAxLTI0ICBIYW5z IFdlbm5ib3JnICA8aGFuc0BjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgRGlzYWJsZSBDbGFuZydz IC1XcmV0dXJuLXR5cGUtYy1saW5rYWdlIGZvciBTb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC8K KyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwNzg0NQor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEEgcmVjZW50 IHZlcnNpb24gb2YgQ2xhbmcgc3RhcnRlZCB3YXJuaW5nIGFib3V0IGNvZGUgaW4KKyAgICAgICAg U291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvbnBydW50aW1lLmNwcC4KKworICAgICAgICBJbiB0 aGlzIGNhc2UgaXQgaXMgd2FybmluZyBhYm91dCBhbiAnZXh0ZXJuICJjIicgZnVuY3Rpb24gcmV0 dXJuaW5nIGEKKyAgICAgICAgbm9uLVBPRCB0eXBlLiBCdXQgYmVjYXVzZSB0aGUgZnVuY3Rpb24g aGFzIGludGVybmFsIGxpbmthZ2UsIHRoZQorICAgICAgICB3YXJuaW5nIGRvZXNuJ3QgcmVhbGx5 IGFwcGx5LgorCisgICAgICAgIFRoaXMgY2hhbmdlIHN1cHByZXNzZXMgdGhlIHdhcm5pbmcgaW4g dGhlIG1lYW50aW1lLgorCisgICAgICAgIFRlc3RlZCBieSBidWlsZGluZyB3aXRoIENsYW5nIHIx NzI0NTQuCisKKyAgICAgICAgKiBXZWJDb3JlLmd5cC9XZWJDb3JlLmd5cDoKKwogMjAxMy0wMS0y NCAgQW5kcmV5IEx1c2huaWtvdiAgPGx1c2huaWtvdkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAg V2ViIEluc3BlY3RvcjogZXhwYW5kIG1vcmUgY2h1bmtzIGluIERURQpkaWZmIC0tZ2l0IGEvU291 cmNlL1dlYkNvcmUvV2ViQ29yZS5neXAvV2ViQ29yZS5neXAgYi9Tb3VyY2UvV2ViQ29yZS9XZWJD b3JlLmd5cC9XZWJDb3JlLmd5cAppbmRleCA0YTdkMDc4MjIzYzQzNjY3MzJhOTI4ODQ4YjVlOTFk NjhmNjc0OTEzLi45MDJmOTdiNjExNzM4YjNlZjVjYTUwZTQxNWUwYWViMzFmMmY2Y2ViIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9XZWJDb3JlLmd5cC9XZWJDb3JlLmd5cAorKysgYi9Tb3Vy Y2UvV2ViQ29yZS9XZWJDb3JlLmd5cC9XZWJDb3JlLmd5cApAQCAtMjE3Niw2ICsyMTc2LDE3IEBA CiAgICAgICAgIFsnT1MhPSJtYWMiJywgewogICAgICAgICAgICdzb3VyY2VzLyc6IFtbJ2V4Y2x1 ZGUnLCAnTWFjXFwuKGNwcHxtbT8pJCddXQogICAgICAgICB9XSwKKyAgICAgICAgWydjbGFuZz09 MScsIHsKKyAgICAgICAgICAjIEZJWE1FOiBSZW1vdmUgb25jZSB0aGlzIHdhcm5pbmcgaGFzIGJl ZW4gdHdlYWtlZCBpbiBDbGFuZy4KKyAgICAgICAgICAnY2ZsYWdzJzogWworICAgICAgICAgICAg Jy1Xbm8tcmV0dXJuLXR5cGUtYy1saW5rYWdlJywKKyAgICAgICAgICBdLAorICAgICAgICAgICd4 Y29kZV9zZXR0aW5ncyc6IHsKKyAgICAgICAgICAgICdXQVJOSU5HX0NGTEFHUyc6IFsKKyAgICAg ICAgICAgICAgJy1Xbm8tcmV0dXJuLXR5cGUtYy1saW5rYWdlJywKKyAgICAgICAgICAgIF0sCisg ICAgICAgICAgfQorICAgICAgICB9XSwKICAgICAgIF0sCiAgICAgfSwKICAgICB7Cg==