837313 - Mark some more things in /image/decoders as FAIL_ON_WARNINGS

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Daniel Holbert [:dholbert]]

These directories are warning-free [1], but not yet marked as FAIL_ON_WARNINGS:
 image/decoders
 image/decoders/icon
 image/decoders/icon/win

[1] image/decoders actually has two MSVC "warning C4244...possible loss of data" for converting from a wider type to a shorter type, so I'm exempting MSVC from that directory's annotation for now. (This doesn't affect the other directories)

Comment 1

[Daniel Holbert [:dholbert]]

Note:
 * /icon/gtk and /icon/android area already annotated as FAIL_ON_WARNINGS. 
 * /icon/mac only has a .mm file, which I don't think (?) FAIL_ON_WARNINGS has any effect on, so I didn't bother with that one
 * The other /icon subdirs are for non-TBPL platforms, so I don't want to impose anything on them, lest they break without us noticing.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> [1] image/decoders actually has two MSVC "warning C4244...possible loss of
> data" for converting from a wider type to a shorter type, so I'm exempting
> MSVC from that directory's annotation for now. (This doesn't affect the
> other directories)

(for reference, those MSVC warnings are:
e:/builds/moz2_slave/try-w32/build/image/decoders/nsPNGDecoder.cpp(461) : warning C4244: 'argument' : conversion from 'double' to 'float', possible loss of data
e:/builds/moz2_slave/try-w32/build/image/decoders/nsGIFDecoder2.cpp(189) : warning C4244: 'argument' : conversion from 'uint16_t' to 'uint8_t', possible loss of data
  https://tbpl.mozilla.org/php/getParsedLog.php?id=19368821&tree=Try#error0
I'm deliberately not worrying about those in this bug. We can fix them elsewhere if they're important.)

Comment 4

[Joe Drew (not getting mail)]

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

Review of attachment 709268 [details] [diff] [review]:
-----------------------------------------------------------------

::: image/decoders/Makefile.in
@@ +16,5 @@
>  MODULE_NAME = nsDecodersModule
>  LIBXUL_LIBRARY  = 1
> +ifndef _MSC_VER
> +FAIL_ON_WARNINGS = 1
> +endif # !_MSC_VER

the "# !_MSC_VER" might be overkill

Comment 5

[Daniel Holbert [:dholbert]]

Yeah... That comment is just cargo-culted from bug 824247 patch 3 (which added a bunch of tree-wide "_MSC_VER" checks around FAIL_ON_WARNINGS annotations) -- so I'll lean towards keeping it, for consistency, but I'm happy to remove it if you have strong objections. :)

Thanks!

Comment 6

[Daniel Holbert [:dholbert]]

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

Comment 7

[Phil Ringnalda (:philor)]

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