832545 - Mark /gfx/thebes as FAIL_ON_WARNINGS (but not on MSVC, for 'loss of precision' warnings)

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Daniel Holbert [:dholbert]]

With bug 832440, bug 832486, and bug 832456 fixed, /gfx/thebes is now warning-free on my Linux machine.

Filing this bug on marking it as such.

(We can't mark this directory as warning-free on Windows/MSVC, because there are a lot of MSVC-only 'loss of precision' warnings for e.g. double --> float conversions, which I don't think are going to be fixed anytime soon.)

Comment 1

[Daniel Holbert [:dholbert]]

I did a Try push to test this out:
 https://tbpl.mozilla.org/?tree=Try&rev=cd68a7fafbcd

It hit mac failures due to using a deprecated API in some mac-specific code.  I filed bug 832548 on that, in case it's easily-fixable -- but if we're stuck using that deprecated API (and hence stuck with the build warning), we may want to make the Makefile annotation non-Mac (as well as non-MSVC).

Comment 2

[Daniel Holbert [:dholbert]]

Try push with the various android-only thebes build warnings fixed (I think):
 https://tbpl.mozilla.org/?tree=Try&rev=5837079c554e

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #1)
> It hit mac failures due to using a deprecated API in some mac-specific code.
> [...] if we're
> stuck using that deprecated API (and hence stuck with the build warning), we
> may want to make the Makefile annotation non-Mac (as well as non-MSVC).

(actually, I think we may want to keep that build warning from triggering errors -- I filed bug 833405 on that.)

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: fix

Green try push (w/ bug 832440 fixed), demonstrating that this doesn't bust stuff:
 https://tbpl.mozilla.org/?tree=Try&rev=9c2797db6bb3

Comment 5

[Benoit Girard (:BenWa)]

Comment on attachment 705159 [details] [diff] [review]
fix

I'm not to familiar with the right ifdef variable (_MSC_VER) to use but given that it's used elsewhere I don't think it's worth getting a build peer to review it.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Benoit Girard (:BenWa) from comment #5)
> I'm not to familiar with the right ifdef variable (_MSC_VER) to use but
> given that it's used elsewhere I don't think it's worth getting a build peer
> to review it.

That's the variable that was used in bug 824247 part 3, which Ted reviewed, so I think it's good.

Thanks for the review!

Comment 7

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cdd57f3e1384

Comment 8

[Daniel Holbert [:dholbert]]

all-green try run from earlier today, FWIW (w/ fix for recently-introduced bug 835473):
 https://tbpl.mozilla.org/?tree=Try&rev=808ba8a1342e

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cdd57f3e1384