This patch kills 30+ build warnings in mozilla/js/src. I believe that these are the correct resolutions for these warnings. All of them are uninitialized variable warnings, and all of the ones which I fixed were bogus. There is one legitimate warning, where we are using a pointer which could be garbage. I have added a FIXME comment to the source in that place, because I don't know the right way to fix it. It is possible that we never crash there, but the could should be more explicit. If you are cc on this bug, that means that tinderbox blamed you.

I'm not very happy with having the software burn cycles because this particular compiler is not smart enough to see that code of the form... int i; if(foo) i = 1; else i = 2; bar(i); ...does not use any uninitialized variables.

I won't take this patch, but I feel bad writing that since you did work to come up with it and attach it. These warnings are bogus, as you wrote (if what you meant was, these warning are false alarms). What's more, the FIXME comment you attached is not a bug: notice the || expression governing the preceding if-then statement, and the early return at the end of the then block. aobj will be set if control reaches the successor block to the if-then. I keep a close eye on warnings produced by gcc debug builds of JS, but I can't abide the optimizer's crying wolf. Any modern optimizing back end (SSA based) would know better. BTW, the patch in bug 49816 fixes the jsparse.c warnings, with conforming style (the JS engine never uses foo=42, always putting spaces around operators) and in the case of op = JSOP_NOP, doing that only in the default: case of the switch, not in an initializer. Let me know if I've missed anything here. Thanks -- I'll mark this WONTFIX tomorrow unless there's something truly broken. /be

Urk, forgot to be clear that the hasDefault warning for jsparse.c is valid: an empty JS switch would lead to a UMR at ok &= hasDefault; (my face is red). But as I wrote, the patch for bug 49816 fixes that. /be

The practical difference between char *this; and char *this = NULL is barely even worth mentioning (1 extra byte of object code) in a product of this scope. My goal with this patch is to go through the build log looking for legitimate warnings and reducing the noise. If you don't want to commit the hush fixes, that's fine with me, but I expect to find at least a few real bugs in the mix, like the one in jsparse.c. BTW Brendan you are right that my FIXME is a false alarm, but there's something wrong when it takes someone > 5 minutes to consider the result of an if statement.

just a question: in your experience, jband, would a compiler ever optimize out an initialization at the point of declaration, had there been one, in your example above? Plainly it's not needed; I've seen optimizers elide assignments into dead variables ...

jwbaker: we have some AI in the tinderbox warning filter that takes out known to be useless warnings (such as those about constant expressions always true or false in the condition of a ?: expression, namely a JS_ASSERT, PR_ASSERT, or similar -- sometimes you need such runtime assertions because the C preprocessor can't handle sizeof). I would hate to teach this filter about a long list of false alarms, but if there's a way to separate the wheat from the chaff reliably.... The JS engine is one of those places where every cycle counts, not that it has been optimized much (over the years, the emphasis has been on completeness and extensibility, not performance; but now there is a fair deal of performance pressure). But performance quibbling aside, the main point is that I think gcc needs to do a better job, and I don't view it as my job to as "how high" when it says "jump" (but I do pay attention, thank you for the jsparse.c reminder -- I'd already found that via my own optimized gcc builds). scc: if the optimizer is claiming that foo may not be set at a use point, then I doubt it will have the wits to remove the unnecessary initialization, were you to add one. Again I say gcc has a bug, not (necessarily) any source that it whines about in the way this bug reports. /be

Marking Verified -

REMIND is deprecated per bug 35839.

If this is gcc's silliness and not ours, INVALID.

Marking Verified Invalid -