[llvm-commits] [llvm] r155307 - in /llvm/trunk/lib: Support/YAMLParser.cpp Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp

Chandler Carruth chandlerc at google.com
Sun Apr 22 00:53:43 PDT 2012


On Sun, Apr 22, 2012 at 12:37 AM, Bill Wendling <isanbard at gmail.com> wrote:

> On Apr 22, 2012, at 12:34 AM, Chandler Carruth wrote:
>
> > On Sun, Apr 22, 2012 at 12:23 AM, Bill Wendling <isanbard at gmail.com>
> wrote:
> > Author: void
> > Date: Sun Apr 22 02:23:04 2012
> > New Revision: 155307
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=155307&view=rev
> > Log:
> > Remove some potential warnings about variables used uninitialized.
> >
> > I'm not thrilled with this change. I know that GCC has a broken
> uninitialized variable warning, but I don't think we should silence it this
> way. When we initialize these variables unnecessarily, we lose the ability
> to track actual bugs with tools like Valgrind.
> >
> > Do these warnings really cause any problems? Newer GCC versions can turn
> them off with -Wno-maybe-uninitialized, and they're off in Clang for the
> same reason...
>
> The .getAsInteger methods can return without assigning a value to the
> variable, so those seem like legitimate errors.


But the fix should be quite different I suspect -- given the TODO on the
previous line to actually report an error in other cases we should probably
do the same here.

And we probably need some more test cases so that Valgrind would actually
detect this error in the future.

I've CC'ed Michael who should probably look at this.


> The only one I would question is the Mips fix, which seems rather odd that
> it could get past the switch statement without assigning a value to the
> variable.
>

This is likely because while the switch "covers" the enumerators, GCC's
warning uses the strict interpretation of an enum's range of possible
values. This is also why GCC's -Wreturn-type fires if we don't put the
'llvm_unreachable(...)' after covering switches where each case returns...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120422/be406f98/attachment.html>


More information about the llvm-commits mailing list