[llvm] r250929 - Silence Visual C++ warning in function summary parsing code (NFC)

[David Blaikie via llvm-commits]

On Wed, Oct 21, 2015 at 2:52 PM, Teresa Johnson <tejohnson at google.com>
wrote:

> On Wed, Oct 21, 2015 at 12:49 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >
> >
> > On Wed, Oct 21, 2015 at 12:33 PM, Aaron Ballman via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >>
> >> On Wed, Oct 21, 2015 at 3:25 PM, Teresa Johnson via llvm-commits
> >> <llvm-commits at lists.llvm.org> wrote:
> >> > Author: tejohnson
> >> > Date: Wed Oct 21 14:25:14 2015
> >> > New Revision: 250929
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=250929&view=rev
> >> > Log:
> >> > Silence Visual C++ warning in function summary parsing code (NFC)
> >> >
> >> > Modified:
> >> >     llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> >> >
> >> > Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=250929&r1=250928&r2=250929&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
> >> > +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Wed Oct 21
> 14:25:14
> >> > 2015
> >> > @@ -5538,6 +5538,7 @@ std::error_code FunctionIndexBitcodeRead
> >> >    // importing is added so that it can be tested.
> >> >    SmallVector<uint64_t, 64> Record;
> >> >    switch (Stream.readRecord(Entry.ID, Record)) {
> >> > +    case bitc::FS_CODE_COMBINED_ENTRY:
> >> >      default:
> >> >        return error("Invalid record");
> >>
> >> What warning is this silencing?
>
> BitcodeReader.cpp(5543): warning C4065: switch statement contains
> 'default' but no 'case' labels.
>
> >> It seems like the correct approach is
> >> to remove the switch entirely.
> >
> >
> > +1 - this looks like it should just be:
> >
> > if (Stream.readRecord(...))
> >   return error(...);
>
> I could do that too, it is currently a placeholder for a TODO when the
> full ThinLTO pipeline is hooked up. The comment just above is:
>
>   // TODO: Read a record. This interface will be completed when ThinLTO
>   // importing is added so that it can be tested.
>
> Eventually it will parse FS_CODE_COMBINED_ENTRY records. If there is a
> strong preference to remove this as Aaron suggested, I can do that
> though.
>

*shrug* not too fussed (& my suggestion's wrong too, of course, I guess it
should be "Stream.readRecord(Entry.ID, Record); return error("Invalid
record");" - no if/condition/control flow/etc), but probably wouldn't hurt
to just strip out the placeholder code for now and leave the FIXME to
describe where this code is going.

*looks at the code some more*
So this function is called currently, but always fails? Is that
tested/useful? (I'm sure it was all reviewed by people with far more
context than me when it went in, so don't let me derail you or anything -
I'm just a tad confused at first glance)


>
> Teresa
>
> >
> >>
> >>
> >> ~Aaron
> >>
> >> >    }
> >> >
> >> >
> >> > _______________________________________________
> >> > llvm-commits mailing list
> >> > llvm-commits at lists.llvm.org
> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> >
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151021/03efa5e0/attachment.html>