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)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b5

People

(Reporter: RyanVM, Assigned: RyanVM)

References

Details

(Whiteboard: [build_warning])

Attachments

(1 file, 1 obsolete file)

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?
Attached patch Cast to floats (obsolete) — Splinter Review
Seems to work.
Assignee: nobody → ryanvm
Status: NEW → ASSIGNED
Attachment #467634 - Flags: review?(roc)
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+
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?
Attachment #467643 - Flags: approval2.0? → approval2.0+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/2d6ead22831c
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b5
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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+
(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)
http://hg.mozilla.org/mozilla-central/rev/5ac01a75e650
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.