Mark gfx/2d as FAIL_ON_WARNINGS

RESOLVED FIXED in mozilla28

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla28
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

5 years ago
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

5 years ago
Created attachment 711544 [details] [diff] [review]
fix v1

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)

Updated

5 years ago
Depends on: 795655
(Assignee)

Updated

5 years ago
Depends on: 839347
(Assignee)

Updated

5 years ago
Depends on: 839383
(Assignee)

Updated

5 years ago
Depends on: 839384
(Assignee)

Comment 2

5 years ago
Created attachment 711731 [details] [diff] [review]
fix v2: FAIL_ON_WARNINGS but not on MSVC

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+
(Assignee)

Updated

5 years ago
Depends on: 857740
(Assignee)

Comment 3

5 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

5 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)

Updated

5 years ago
Depends on: 858274
(Assignee)

Updated

5 years ago
Depends on: 858304
(Assignee)

Comment 5

5 years ago
Created attachment 733603 [details] [diff] [review]
fix v3: FAIL_ON_WARNINGS (now without MSVC exemption)

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)

Updated

4 years ago
Depends on: 924444
(Assignee)

Updated

4 years ago
No longer depends on: 858274
(Assignee)

Comment 6

4 years ago
Created attachment 814714 [details] [diff] [review]
fix v4 (now in moz.build)

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

4 years ago
Attachment #733603 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Depends on: 924749
(Assignee)

Updated

4 years ago
Depends on: 924768
(Assignee)

Comment 7

4 years ago
Try run:
 https://tbpl.mozilla.org/?tree=Try&rev=abb7329ebc02

Landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/a555f881daf6

Comment 8

4 years ago
https://hg.mozilla.org/mozilla-central/rev/a555f881daf6
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.