<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Oct 14, 2013 at 11:20 PM, Andy Gibbs <span dir="ltr"><<a href="mailto:andyg1001@hotmail.co.uk" target="_blank">andyg1001@hotmail.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Monday, October 14, 2013 6:16 PM, David Blaikie wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Generally I believe we would prefer to disable the warning<br>
than needlessly initialize the variable (is this needless<br>
initialization? I haven't looked at the rest of the code,<br>
but I assume it is). That way dynamic tools (MSan, Valgrind,<br>
etc) can properly diagnose when the algorithm has bugs that<br>
fail to initialize this variable.<br>
</blockquote>
<br></div><div><div class="h5">
And on Monday, October 14, 2013 6:33 PM, Robert Lytton wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi David,<br>
<br>
Just to confirm that the initialization was needless.<br>
All enum types are covered in switch statement.<br>
Each case assigns to the variable.<br>
</blockquote>
<br></div></div>
Yes, each case assigns to the variable (even the one with the<br>
llvm_unreachable), but gcc seems to be over-zealous with this<br>
warning and doesn't consider the switch statement to be fully<br>
covered.  I think it happens under some specific optimisation<br>
level, but I don't know the ins and outs of it, this is just<br>
the impression I've gathered over time.<br>
<br>
David, what would be the best best way to disable the warning<br>
here?  I take your point about using dynamic tools, but the<br>
code in its current state will not compile under release mode<br>
with -Werror on gcc.  I wouldn't want to do some sort of<br>
kluge in the order of a diagnostic pragma; would you be happy<br>
with an alternative kludge: a LLVM_WEAK_INITIALIZE(...) macro<br>
inside llvm/Compiler.h that expands to nothing unless using<br>
gcc?  I guess there are loads of places across the whole code<br>
where this might be used; I don't think this is an isolated<br>
case of a technically unnecessary initialisation.</blockquote><div><br></div><div>You're right, this isn't isolated - the solution is to literally disable the warning for the whole project from within our build files. I believe we already have some GCC diagnostics disabled, this would just be another to add to that list.  <br>
</div><div><br></div><div>Chandler (or many other people) might know more about the build system and where this is best handled than I would, my naive guess would be (for the Makefile build, at least - cc'ng Eric as the owner of that) in the "Options To Invoke Tools" section of Makefile.rules, simply CompileCommonOpts += -Wno-maybe-uninitialized. Clang should have a no-op alias for this warning (I haven't checked if it does) and supports more accurate warnings like -Wsometimes-uninitialized (which should already be on in our builds) anyway.<br>
</div></div></div></div>