Closed Bug 839269 Opened 8 years ago Closed 7 years ago

Mark gfx/2d as FAIL_ON_WARNINGS

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file, 3 obsolete files)

W/ dependent bugs fixed, gfx/2d is warning-free on my machine.

Filing this bug on marking it as FAIL_ON_WARNINGS.
Attached patch fix v1 (obsolete) — Splinter Review
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
Depends on: 795655
Depends on: 839347
Depends on: 839383
Depends on: 839384
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)
Attachment #711731 - Flags: review?(jmuizelaar) → review+
Depends on: 857740
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.
(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.
Depends on: 858274
Depends on: 858304
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+
Depends on: 924444
No longer depends on: 858274
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+
Attachment #733603 - Attachment is obsolete: true
Depends on: 924749
Depends on: 924768
https://hg.mozilla.org/mozilla-central/rev/a555f881daf6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.