Closed
Bug 588977
Opened 14 years ago
Closed 14 years ago
warning C4244: 'argument' : conversion from 'const PRInt32' to 'float', possible loss of data in nsPresContext.h
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b5
People
(Reporter: RyanVM, Assigned: RyanVM)
References
Details
(Whiteboard: [build_warning])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
RyanVM
:
review+
sicking
:
approval2.0+
|
Details | Diff | Splinter Review |
The DPI rewrite caused a new warning to be spammed all over when building with MSVC.
http://hg.mozilla.org/mozilla-central/rev/585a75516573
nsPresContext.h(628) : warning C4244: 'argument' : conversion from 'const PRInt32' to 'float', possible loss of data
nsPresContext.h(629) : warning C4244: 'argument' : conversion from 'const PRInt32' to 'float', possible loss of data
nsPresContext.h(630) : warning C4244: 'argument' : conversion from 'const PRInt32' to 'float', possible loss of data
nsPresContext.h(631) : warning C4244: 'argument' : conversion from 'const PRInt32' to 'float', possible loss of data
roc, is it safe to just static cast these?
Yes
Assignee | ||
Comment 2•14 years ago
|
||
Seems to work.
Assignee | ||
Updated•14 years ago
|
Whiteboard: [build_warning]
Comment on attachment 467634 [details] [diff] [review]
Cast to floats
You can actually write these as constructor casts: float(marginInTwips.left) etc
Attachment #467634 -
Flags: review?(roc) → review+
Assignee | ||
Comment 4•14 years ago
|
||
This patch cuts down on MSVC warning spam in every file that includes nsPresContext.h, which is a very large number.
Attachment #467634 -
Attachment is obsolete: true
Attachment #467643 -
Flags: review+
Attachment #467643 -
Flags: approval2.0?
Updated•14 years ago
|
Attachment #467643 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment 5•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b5
This appears to be causing orange on mochitest, so I backed it out.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1282316894.1282317410.13910.gz
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 7•14 years ago
|
||
Comment on attachment 467643 [details] [diff] [review]
Use constructor cast instead of static cast
Sorry, short leash on approvals mean that if they're backed out they can't get back in.
Attachment #467643 -
Flags: approval2.0+
Comment 8•14 years ago
|
||
(In reply to comment #6)
> This appears to be causing orange on mochitest, so I backed it out.
>
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1282316894.1282317410.13910.gz
(In reply to comment #7)
> Comment on attachment 467643 [details] [diff] [review]
> Use constructor cast instead of static cast
>
> Sorry, short leash on approvals mean that if they're backed out they can't get
> back in.
If this really did create a behavioral difference perhaps the build warning here is substantial, and this bug should be investigated more. Otherwise, this bug is pretty much NPOTB and shouldn't change a thing (besides less compiler spam).
So it appears that this was not the cause of the orange, but rather probably a machine issue. It did seem like a long-shot that this bug was at fault, but it failed at least two builds in a row in exactly the same way.
I'll restore the approved flag so feel free to reland when you can. (I might even be able to take it as a ride-along patch myself)
Attachment #467643 -
Flags: approval2.0+
Comment 10•14 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•