115047 2013-04-23 10:52:22 -0700 RetainPtr.h should compile with -Wshorten-64-to-32 2013-04-23 12:14:32 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 107093 1 ddkilzer ddkilzer andersca benjamin cmarcelo commit-queue darin kling mitz mjs sam oldest_to_newest 879974 0 ddkilzer 2013-04-23 10:52:22 -0700 RetainPtr.h:318:20: error: implicit conversion loses integer precision: 'CFHashCode' (aka 'unsigned long') to 'unsigned int' [-Werror,-Wshorten-64-to-32] return CFHash(o.get()); ~~~~~~ ^~~~~~~~~~~~~~~ CFHash() returns a CFHashCode, which is typedef-ed to 'unsigned long', which is 4 bytes on 32-bit architectures (which matches 'unsigned'), but 8 bytes on 64-bit architectures. I see two possible fixes here: 1. Use a static_cast<uint32_t>() operator to just take the lower 4 bytes for the hash code. This is what is implicitly happening now. 2. Use static_cast<uint64_t>() with WTF::intHash() to generate a new 4-byte hash code using the 8-byte CFHashCode. Thoughts? 880050 1 199310 ddkilzer 2013-04-23 11:34:15 -0700 Created attachment 199310 Patch v1 880052 2 199310 andersca 2013-04-23 11:39:28 -0700 Comment on attachment 199310 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=199310&action=review > Source/WTF/wtf/RetainPtr.h:318 > + return static_cast<uint32_t>(CFHash(o.get())); Wouldn’t it be better to cast to the function's return type (unsigned)? 880053 3 ddkilzer 2013-04-23 11:40:54 -0700 (In reply to comment #2) > (From update of attachment 199310 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=199310&action=review > > > Source/WTF/wtf/RetainPtr.h:318 > > + return static_cast<uint32_t>(CFHash(o.get())); > > Wouldn’t it be better to cast to the function's return type (unsigned)? Yes. 880056 4 199311 ddkilzer 2013-04-23 11:42:34 -0700 Created attachment 199311 Patch v2 880068 5 199311 commit-queue 2013-04-23 12:14:29 -0700 Comment on attachment 199311 Patch v2 Clearing flags on attachment: 199311 Committed r148979: <http://trac.webkit.org/changeset/148979> 880069 6 commit-queue 2013-04-23 12:14:32 -0700 All reviewed patches have been landed. Closing bug. 199310 2013-04-23 11:34:15 -0700 2013-04-23 11:42:26 -0700 Patch v1 bug-115047-20130423113340.patch text/plain 1783 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ4OTQwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDBkNmExOWMzMjUwYzE2YjU1MDllNDMy OGQyZjc5Y2ViY2M5ZGMwMzkuLjUxNDkxZWY3M2NjNTFiZjQwZWMzYmYyM2U0N2Y3MjA3Y2UzZmU3 MzEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMjIgQEAKKzIwMTMtMDQtMjMgIERhdmlkIEtpbHplciAgPGRka2ls emVyQGFwcGxlLmNvbT4KKworICAgICAgICBSZXRhaW5QdHIuaCBzaG91bGQgY29tcGlsZSB3aXRo IC1Xc2hvcnRlbi02NC10by0zMgorICAgICAgICA8aHR0cDovL3dlYmtpdC5vcmcvYi8xMTUwNDc+ CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4ZXMg dGhlIGZvbGxvd2luZyB3YXJuaW5nOgorCisgICAgICAgICAgICBSZXRhaW5QdHIuaDozMTg6MjA6 IGVycm9yOiBpbXBsaWNpdCBjb252ZXJzaW9uIGxvc2VzIGludGVnZXIgcHJlY2lzaW9uOiAnQ0ZI YXNoQ29kZScgKGFrYSAndW5zaWduZWQgbG9uZycpIHRvICd1bnNpZ25lZCBpbnQnIFstV2Vycm9y LC1Xc2hvcnRlbi02NC10by0zMl0KKyAgICAgICAgICAgICAgICAgICAgICAgIHJldHVybiBDRkhh c2goby5nZXQoKSk7CisgICAgICAgICAgICAgICAgICAgICAgICB+fn5+fn4gXn5+fn5+fn5+fn5+ fn5+CisKKyAgICAgICAgKiB3dGYvUmV0YWluUHRyLmg6CisgICAgICAgIChXVEY6OlJldGFpblB0 ck9iamVjdEhhc2g6Omhhc2gpOiBDYXN0IHJldHVybiB2YWx1ZSBvZiBDRkhhc2goKQorICAgICAg ICB0byB1aW50MzJfdC4gIFRoaXMgaGFzIG5vIGVmZmVjdCBvbiAzMi1iaXQgYXJjaGl0ZWN0dXJl cywgYnV0CisgICAgICAgIHRha2VzIHRoZSBsb3dlciAzMi1iaXRzIG9mIHRoZSBDRkhhc2hDb2Rl ICh1bnNpZ25lZCBsb25nKSBvbgorICAgICAgICA2NC1iaXQgYXJjaGl0ZWN0dXJlcywgd2hpY2gg aXMgd2hhdCBoYXBwZW5zIGltcGxpY2l0bHkgbm93LgorCiAyMDEzLTA0LTIyICBEYXZpZCBLaWx6 ZXIgIDxkZGtpbHplckBhcHBsZS5jb20+CiAKICAgICAgICAgU3RyaW5nSW1wbC5oIHNob3VsZCBj b21waWxlIHdpdGggLVdzaG9ydGVuLTY0LXRvLTMyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL3d0 Zi9SZXRhaW5QdHIuaCBiL1NvdXJjZS9XVEYvd3RmL1JldGFpblB0ci5oCmluZGV4IDk5ODFhNGQ5 ODhhOGEzNWQ0ZjkzZGQyMjZiOTU5ZWMwYmJlNTc2YzIuLmMxNGJmNmIxZWI1ZTlhZGYyMDExZmNh YjRjNjQwMzcxMWZkZmRjMDAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL1JldGFpblB0ci5o CisrKyBiL1NvdXJjZS9XVEYvd3RmL1JldGFpblB0ci5oCkBAIC0zMTUsNyArMzE1LDcgQEAgbmFt ZXNwYWNlIFdURiB7CiAgICAgICAgIHN0YXRpYyB1bnNpZ25lZCBoYXNoKGNvbnN0IFJldGFpblB0 cjxQPiYgbykKICAgICAgICAgewogICAgICAgICAgICAgQVNTRVJUX1dJVEhfTUVTU0FHRShvLmdl dCgpLCAiYXR0ZW1wdCB0byB1c2UgbnVsbCBSZXRhaW5QdHIgaW4gSGFzaFRhYmxlIik7Ci0gICAg ICAgICAgICByZXR1cm4gQ0ZIYXNoKG8uZ2V0KCkpOworICAgICAgICAgICAgcmV0dXJuIHN0YXRp Y19jYXN0PHVpbnQzMl90PihDRkhhc2goby5nZXQoKSkpOwogICAgICAgICB9CiAgICAgICAgIHN0 YXRpYyBib29sIGVxdWFsKGNvbnN0IFJldGFpblB0cjxQPiYgYSwgY29uc3QgUmV0YWluUHRyPFA+ JiBiKQogICAgICAgICB7Cg== 199311 2013-04-23 11:42:34 -0700 2013-04-23 12:14:29 -0700 Patch v2 bug-115047-20130423114158.patch text/plain 1783 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ4OTQwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDBkNmExOWMzMjUwYzE2YjU1MDllNDMy OGQyZjc5Y2ViY2M5ZGMwMzkuLjExODhkOTU2ZDViOTZhOWIxODYxMzVjODBkZjdiOWZhNGY1YTU2 M2MgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMjIgQEAKKzIwMTMtMDQtMjMgIERhdmlkIEtpbHplciAgPGRka2ls emVyQGFwcGxlLmNvbT4KKworICAgICAgICBSZXRhaW5QdHIuaCBzaG91bGQgY29tcGlsZSB3aXRo IC1Xc2hvcnRlbi02NC10by0zMgorICAgICAgICA8aHR0cDovL3dlYmtpdC5vcmcvYi8xMTUwNDc+ CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4ZXMg dGhlIGZvbGxvd2luZyB3YXJuaW5nOgorCisgICAgICAgICAgICBSZXRhaW5QdHIuaDozMTg6MjA6 IGVycm9yOiBpbXBsaWNpdCBjb252ZXJzaW9uIGxvc2VzIGludGVnZXIgcHJlY2lzaW9uOiAnQ0ZI YXNoQ29kZScgKGFrYSAndW5zaWduZWQgbG9uZycpIHRvICd1bnNpZ25lZCBpbnQnIFstV2Vycm9y LC1Xc2hvcnRlbi02NC10by0zMl0KKyAgICAgICAgICAgICAgICAgICAgICAgIHJldHVybiBDRkhh c2goby5nZXQoKSk7CisgICAgICAgICAgICAgICAgICAgICAgICB+fn5+fn4gXn5+fn5+fn5+fn5+ fn5+CisKKyAgICAgICAgKiB3dGYvUmV0YWluUHRyLmg6CisgICAgICAgIChXVEY6OlJldGFpblB0 ck9iamVjdEhhc2g6Omhhc2gpOiBDYXN0IHJldHVybiB2YWx1ZSBvZiBDRkhhc2goKQorICAgICAg ICB0byB1bnNpZ25lZC4gIFRoaXMgaGFzIG5vIGVmZmVjdCBvbiAzMi1iaXQgYXJjaGl0ZWN0dXJl cywgYnV0CisgICAgICAgIHRha2VzIHRoZSBsb3dlciAzMi1iaXRzIG9mIHRoZSBDRkhhc2hDb2Rl ICh1bnNpZ25lZCBsb25nKSBvbgorICAgICAgICA2NC1iaXQgYXJjaGl0ZWN0dXJlcywgd2hpY2gg aXMgd2hhdCBoYXBwZW5zIGltcGxpY2l0bHkgbm93LgorCiAyMDEzLTA0LTIyICBEYXZpZCBLaWx6 ZXIgIDxkZGtpbHplckBhcHBsZS5jb20+CiAKICAgICAgICAgU3RyaW5nSW1wbC5oIHNob3VsZCBj b21waWxlIHdpdGggLVdzaG9ydGVuLTY0LXRvLTMyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL3d0 Zi9SZXRhaW5QdHIuaCBiL1NvdXJjZS9XVEYvd3RmL1JldGFpblB0ci5oCmluZGV4IDk5ODFhNGQ5 ODhhOGEzNWQ0ZjkzZGQyMjZiOTU5ZWMwYmJlNTc2YzIuLjY5Nzg0OWY3YTAwN2VjNzhhNGE3NTM1 MmQzMTg0MDJlOWFiMWM0MDggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL1JldGFpblB0ci5o CisrKyBiL1NvdXJjZS9XVEYvd3RmL1JldGFpblB0ci5oCkBAIC0zMTUsNyArMzE1LDcgQEAgbmFt ZXNwYWNlIFdURiB7CiAgICAgICAgIHN0YXRpYyB1bnNpZ25lZCBoYXNoKGNvbnN0IFJldGFpblB0 cjxQPiYgbykKICAgICAgICAgewogICAgICAgICAgICAgQVNTRVJUX1dJVEhfTUVTU0FHRShvLmdl dCgpLCAiYXR0ZW1wdCB0byB1c2UgbnVsbCBSZXRhaW5QdHIgaW4gSGFzaFRhYmxlIik7Ci0gICAg ICAgICAgICByZXR1cm4gQ0ZIYXNoKG8uZ2V0KCkpOworICAgICAgICAgICAgcmV0dXJuIHN0YXRp Y19jYXN0PHVuc2lnbmVkPihDRkhhc2goby5nZXQoKSkpOwogICAgICAgICB9CiAgICAgICAgIHN0 YXRpYyBib29sIGVxdWFsKGNvbnN0IFJldGFpblB0cjxQPiYgYSwgY29uc3QgUmV0YWluUHRyPFA+ JiBiKQogICAgICAgICB7Cg==