Closed
Bug 839269
Opened 12 years ago
Closed 11 years ago
Mark gfx/2d as FAIL_ON_WARNINGS
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla28
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
W/ dependent bugs fixed, gfx/2d is warning-free on my machine.
Filing this bug on marking it as FAIL_ON_WARNINGS.
Assignee | ||
Comment 1•12 years ago
|
||
Here's the patch.
Not requesting review until I've got Try feedback, since there are probably a few other warnings hiding on our other platforms/compilers.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•12 years ago
|
||
First try run (w/ fix v1):
https://tbpl.mozilla.org/?tree=Try&rev=1115ce9503fd
On MSVC debug, that encountered some C4244 warnings for converting between floating-point types. There are probably more of these, and they're often annoying/hacky to fix, so I'm just making this non-MSVC for now.
The try run also encountered some mac-specific (and clang-specific) warnings, which I filed new bugs for. Here's a try run with patches (or a partial patch, in one case) for all of those:
https://tbpl.mozilla.org/?tree=Try&rev=0639787d493f
All green, woot!
Attachment #711544 -
Attachment is obsolete: true
Attachment #711731 -
Flags: review?(jmuizelaar)
Updated•12 years ago
|
Attachment #711731 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 3•12 years ago
|
||
I verified that this compiles successfully on my local machine with GCC 4.7, now that bug 839383 has been resolved.
However, my GCC 4.8 prerelease does produce one additional warning in gfx/2d (which this bug's patch treats as an error) -- I'd like to fix that warning before landing, so that this doesn't bust GCC 4.8 builds. I filed bug 857740 on fixing that warning, with a patch.
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #2)
> On MSVC debug, that encountered some C4244 warnings for converting between
> floating-point types. There are probably more of these, and they're often
> annoying/hacky to fix, so I'm just making this non-MSVC for now.
Bug 857863 is actually disabling C4244, which removes the need for the MSVC exemption here. So, once the tree's reopened, I'll land this (after Bug 857863) without the MSVC exemption.
Assignee | ||
Comment 5•12 years ago
|
||
Darn, we've got some new mac-specific warnings in this directory. I filed bug 858274 and bug 858304 for those.
Once those are addressed, here's the final ready-to-land patch for this bug (with the MSVC exemption removed, per comment 4). Try run (green aside from the mac issues):
https://tbpl.mozilla.org/?tree=Try&rev=28a74a7ddfd4
Attachment #711731 -
Attachment is obsolete: true
Attachment #733603 -
Flags: review+
Assignee | ||
Comment 6•11 years ago
|
||
Now that the (at one time) last warnings for this dir, in QuartzSupport.mm, are being fixed over in bug 924444, here's an updated version of the FAIL_ON_WARNINGS patch. (now adding the annotation to moz.build instead of Makefile.in, per bug 882859)
Carrying forward r+
Attachment #814714 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Attachment #733603 -
Attachment is obsolete: true
Assignee | ||
Comment 7•11 years ago
|
||
Comment 8•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in
before you can comment on or make changes to this bug.
Description
•