Mark some more things in /image/decoders as FAIL_ON_WARNINGS

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla21
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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)
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.
Posted patch fixSplinter Review
Attachment #709268 - Flags: review?
Attachment #709268 - Flags: review? → review?(joe)
(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 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
Attachment #709268 - Flags: review?(joe) → review+
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!
https://hg.mozilla.org/mozilla-central/rev/e649c4c4cb6f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Depends on: FAIL_ON_WARNINGS
You need to log in before you can comment on or make changes to this bug.