Bug 115047 - RetainPtr.h should compile with -Wshorten-64-to-32
Summary: RetainPtr.h should compile with -Wshorten-64-to-32
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords:
Depends on:
Blocks: 107093
  Show dependency treegraph
 
Reported: 2013-04-23 10:52 PDT by David Kilzer (:ddkilzer)
Modified: 2013-04-23 12:14 PDT (History)
9 users (show)

See Also:


Attachments
Patch v1 (1.74 KB, patch)
2013-04-23 11:34 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch v2 (1.74 KB, patch)
2013-04-23 11:42 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2013-04-23 10:52:22 PDT
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?
Comment 1 David Kilzer (:ddkilzer) 2013-04-23 11:34:15 PDT
Created attachment 199310 [details]
Patch v1
Comment 2 Anders Carlsson 2013-04-23 11:39:28 PDT
Comment on attachment 199310 [details]
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)?
Comment 3 David Kilzer (:ddkilzer) 2013-04-23 11:40:54 PDT
(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.
Comment 4 David Kilzer (:ddkilzer) 2013-04-23 11:42:34 PDT
Created attachment 199311 [details]
Patch v2
Comment 5 WebKit Commit Bot 2013-04-23 12:14:29 PDT
Comment on attachment 199311 [details]
Patch v2

Clearing flags on attachment: 199311

Committed r148979: <http://trac.webkit.org/changeset/148979>
Comment 6 WebKit Commit Bot 2013-04-23 12:14:32 PDT
All reviewed patches have been landed.  Closing bug.