MSVC C4244 is by far our spammiest MSVC warning -- it's already disabled in /js/src due to its low value::trouble ratio. Once the tree reopens, I'll be disabling it in the toplevel configure.in -- more info on that here: https://bugzilla.mozilla.org/show_bug.cgi?id=857863 (There doesn't seem to be a way to keep it enabled but force it to be treated as a warning, as I'd suggested in my previous post, so we're just disabling it.)
At that point, we'll hopefully be able to address the MSVC FAIL_ON_WARNINGS parity issues, here: https://bugzilla.mozilla.org/show_bug.cgi?id=858224 ~Daniel On 04/03/2013 04:56 PM, Daniel Holbert wrote: > Stepping back: [ > This issue is really a special case of "This patch compiles fine in my > local configuration, but it busts the build for $OTHER_PLATFORM". > > The general solution to this class of problems is a try push, with > builds on at least one platform other than your local config (if not all > platforms). That should catch the vast majority of these cases (both > warning-related and non-warning-related), and I think it's reasonable to > expect that non-trivial patches should generally receive a try push > along those lines before landing. > ] > > Now -- for the specific point you brought up about MSVC being excluded > from FAIL_ON_WARNINGS in some directories -- that indeed is undesirable, > and makes it much easier to hit this sort of thing for non-try-tested > pushes. (Usually the MSVC exemption is there because of MSVC-only > warnings for double --> float or float --> int conversions, which we > apparently run afoul of all over the place :-/ ) > > I absolutely agree we should do something to address those and remove > the MSVC-special-casing there. Ideally we'd fix the code so it doesn't > warn, but that's not easy and maybe not necessary in many cases where > this warning might be a false-positive. > > IMHO, the sanest way to address this specific issue would be to use > compiler flags to force the spammiest MSVC-only warnings to be treated > as warnings (not errors), even in FAIL_ON_WARNINGS directories. We > already do this for at least one GCC warning (-Wuninitialized, which has > a lot of false positives.) > > With this change, we could remove most/all of the MSVC exemptions and > get MSVC build-warning-coverage in those directories, so you'd be better > able to locally catch future instances of this sort of issue. > > Would that address your concerns? > > ~Daniel > > P.S. Just to provide the relevant keywords, for MXR searchability & > completeness for anyone who's missing the context here: > * --enable-warnings-as-errors is the mozconfig option that will make > your build treat warnings as errors, for any opted-in directories. > * FAIL_ON_WARNINGS is the Makefile variable used to opt-in directories. > > On 04/03/2013 11:48 AM, Kyle Huey wrote: >> (Ignoring whether or not WARNINGS_AS_ERRORS is a good idea, ignoring >> whether or not having it disabled by default on developer builds but >> enabled on automation is a good idea, ignoring whether or not we can deal >> with the tendency of gcc to lurch from complaining incessantly about one >> silly thing to complaining about another on version upgrades, ignoring the >> differences in warnings between different compilers ..) >> >> The lack of platform parity for WARNINGS_AS_ERRORS being enabled at all >> makes developing on Windows a complete PITA. There are 50+ directories[0] >> that have WARNINGS_AS_ERRORS enabled for gcc/clang and not MSVC. So I come >> along and write a patch that builds just fine on my machine, and push it to >> inbound and every other platform is broken. Now I have to push pretty much >> every patch I write to try (or I just have to disable WARNINGS_AS_ERRORS in >> directories where problems occur). >> >> Either this parity issue needs to be fixed or we need to turn off >> WARNINGS_AS_ERRORS. >> >> - Kyle >> >> [0] http://hg.mozilla.org/mozilla-central/rev/4983b42d58c9 >> _______________________________________________ >> dev-platform mailing list >> dev-platform@lists.mozilla.org >> https://lists.mozilla.org/listinfo/dev-platform >> > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
