Newer version of gcc/g++ (my 4.8.1 build, at least) have a warning "Wmaybe-uninitialized", which has false positives (I hit one today, causing local build bustage, in bug 862115 comment 15). I hit additional build bustage from this warning when I try to do a local opt build (since apparently their rules for what constitutes maybe-uninitialized-usage depends on the optimizations that are enabled). HISTORICAL NOTE: Per http://gcc.gnu.org/ml/gcc/2013-07/msg00177.html , it sounds like Wuninitialized was actually split into Wuninitialized and Wmaybe-uninitialized (with the latter sounding like the fuzzier more-false-positive-ish version) (though I haven't been able to find additional documentation of this). Anyway: since we exempt Wuninitialized from FAIL_ON_WARNINGS, we should presumably exempt Wmaybe-uninitialized, too.

(In reply to Daniel Holbert [:dholbert] from comment #0) > Anyway: since we exempt Wuninitialized from FAIL_ON_WARNINGS, we should > presumably exempt Wmaybe-uninitialized, too. (For reference - that change happened in bug 716541)

Comment on attachment 788243 [details] [diff] [review] fix v1 Review of attachment 788243 [details] [diff] [review]: ----------------------------------------------------------------- Thank you for the detailed history lesson: it made granting r+ much easier! It'd be nice if configure.in contained a similar message saying it should likely be treated the same as -Wuninitialized.

Thanks for the quick review! One clarification question: I don't understand what "similar message" you're asking for. Do you mean I should add a message saying that Wuninitialized and Wmaybe-uninitialized should have the same fate (so if we eventually turn one back on, we should turn the other back on too?) 'cause I'm not sure that's the case -- from my reading, it sounds like (in newer GCC versions) Wuninitialized has become more robust (and might be worth eventually re-enabling for FAIL_ON_WARNINGS), and the false-positive-ish cases have been pushed to Wmaybe-uninitialized.

In any case, I landed the patch to keep m-i buildable for folks like me on GCC 4.8 w/ --enable-warnings-as-errors: https://hg.mozilla.org/integration/mozilla-inbound/rev/b707277861fc (If you'd still like, I can add a comment as a DONTBUILD followup, once I understand what sort of comment you were looking for, per comment 4.)

(In reply to Daniel Holbert [:dholbert] from comment #4) > Thanks for the quick review! > > One clarification question: I don't understand what "similar message" you're > asking for. Do you mean I should add a message saying that Wuninitialized > and Wmaybe-uninitialized should have the same fate (so if we eventually turn > one back on, we should turn the other back on too?) 'cause I'm not sure > that's the case -- from my reading, it sounds like (in newer GCC versions) > Wuninitialized has become more robust (and might be worth eventually > re-enabling for FAIL_ON_WARNINGS), and the false-positive-ish cases have > been pushed to Wmaybe-uninitialized. You are talking a lot more sense than me today. Don't worry about adding a comment.

Cool, thanks. :)

Darn, apparently our OS X compiler (clang version whatever) doesn't have a -Wmaybe-uninitialized option, so it explodes with the current patch. { error: unknown warning option '-Werror=maybe-uninitialized'; did you mean '-Werror=uninitialized'? [-Werror,-Wunknown-warning-option] } https://tbpl.mozilla.org/php/getParsedLog.php?id=26362790&tree=Mozilla-Inbound Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/5ba1323b3806 This probably just needs the appropriate set of conditional wrappers.

Either we upgrade all the compilers or we add a configure check to only add the flag if the compiler supports it. The latter is preferred, otherwise we break a lot of local developer builds still running an old version.

I'd definitely prefer a configure check, too. The point of this is to *fix* local bustage; I don't want to introduce other bustage as collateral damage. :)

We already have the MOZ_C_SUPPORTS_WARNING and MOZ_CXX_SUPPORTS_WARNING macros in build/autoconf/compiler-opts.m4 for conditionally adding flags (if supported) to _WARNINGS_CFLAGS and _WARNINGS_CXXFLAGS. You'll probably need to add variants of those that do the same with WARNINGS_AS_ERRORS. Or generalize the existing macros to work with different variables, if you are an m4 hacker :) (And don't forget to update js/src/build/autoconf/compiler-opts.m4 as well.)

Awesome! We can use those macros (no need to generify/copy them) to handle all of these exemptions. I filed bug 903663 to switch over the existing exemptions, and I'll attach a patch for this bug's exemption.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=2f907df238ba I'll re-request review if that goes well.

Rebasing on top of reviewed patch from bug 903663, now using MOZ_C_SUPPORTS_WARNING() and MOZ_CXX_SUPPORTS_WARNING().

Green try run, for fix v3: https://tbpl.mozilla.org/?tree=Try&rev=d08d00f6da0d