[lld] r264945 - Remove useless unreachable. Switch coverage already gives us this. NFC

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 30 15:49:59 PDT 2016


On Wed, Mar 30, 2016 at 3:47 PM, Mehdi Amini via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
> > On Mar 30, 2016, at 3:34 PM, Pete Cooper via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >
> > Author: pete
> > Date: Wed Mar 30 17:34:37 2016
> > New Revision: 264945
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=264945&view=rev
> > Log:
> > Remove useless unreachable.  Switch coverage already gives us this.  NFC
>
> Does the optimizer has a notion of the switch coverage here? Or is the
> front-end generating an unreachable after your change?
>

In GCC, the issue is that they are pessimistic about enum values - they
assume (technically correct, of course) that an enum can have any of its
representable values, which means that it could contain a value that isn't
one of the enumerators & thus the switch could be bypassed entirely and so
a return after the switch is needed.

Clang avoids the possible false positive by being optimistic and assuming
that the enum will only contain enumerator values.


> Also it is not totally NFC as llvm_unreachable is lowered to a real check
> in an assert build.
>

I think that still qualifies under what I'd consider NFC. An assertion
firing isn't functionality, as such - just bugs.


>
> --
> Mehdi
>
>
>
> >
> > Modified:
> >    lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
> >
> > Modified:
> lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp?rev=264945&r1=264944&r2=264945&view=diff
> >
> ==============================================================================
> > --- lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
> (original)
> > +++ lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
> Wed Mar 30 17:34:37 2016
> > @@ -865,7 +865,6 @@ std::error_code Util::getSymbolTableRegi
> >     }
> >     break;
> >   }
> > -  llvm_unreachable("atom->scope() unknown enum value");
> > }
> >
> > std::error_code Util::addSymbols(const lld::File &atomFile,
> >
> >
> > _______________________________________________
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160330/8875d920/attachment.html>


More information about the llvm-commits mailing list