Closed Bug 670461 Opened 8 years ago Closed 8 years ago
Fix conversion from 'double' to 'PRUint32' build warning in gfx/src/ns
Rendering Context .h(73)
Shows up 144 times in my MSVC2010 local build... /dist/include/nsRenderingContext.h(73) : warning C4244: 'return' : conversion from 'double' to 'PRInt32', possible loss of data http://mxr.mozilla.org/mozilla-central/source/gfx/src/nsRenderingContext.h#73
Replies upon the double overload being added in bug 670338. Thanks :-)
Attachment #545014 - Flags: review?(joe)
Comment on attachment 545014 [details] [diff] [review] Patch v1 Why is mP2A stored as a double if we only use it as a PRInt32? (If there's a good reason, please rerequest review on this patch.)
Attachment #545014 - Flags: review?(joe) → review-
I have no idea (and had thought the same myself, but presumed I was just missing something). So would you like me to just change it to PRInt32?
Presuming the answer to comment 5 is yes, this updated patch stores mP2A as PRInt32 rather than a double, silencing the build warning.
(In reply to comment #6) > http://dev.philringnalda.com/tbpl/?tree=Try&rev=145f59772117 Failed try, by causing reftest failures on all platforms along the lines of: REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/test/build/reftest/tests/layout/reftests/pixel-rounding/iframe-1.html | image comparison (==) :-/
Comment on attachment 546317 [details] [diff] [review] Patch v2 Given try results, clearly there's a reason!
Attachment #546317 - Flags: review?(joe) → review-
Joe, I may have just messed up the v2 patch, given there are other instances where the variable mP2A are used (not all in scope admittedly): http://mxr.mozilla.org/mozilla-central/search?string=mP2A&case=1&tree=mozilla-central Could you take a quick look before we resort to casting, in case changing mP2A to PRInt32 is still a better approach? Thanks! :-)
Joe, any ideas about comment 9? Thanks :-)
Ah, yes - you're clearly right that changing the type is wrong, since it's used internally in nsRenderingContext as a double. Your first patch is therefore correct!
Summary: Fix conversion from 'double' to 'PRInt32' build warning in gfx/src/nsRenderingContext.h(73) → Fix conversion from 'double' to 'PRUint32' build warning in gfx/src/nsRenderingContext.h(73)
Great, thanks Joe :-) Patch for checkin based on the v1 approach, but updated to tip to take account of bug 678222 landing (which changed the return type of AppUnitsPerDevPixel() to be PRUint32).
Whiteboard: [build_warning] → [build_warning][inbound]
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.