[llvm] r305144 - Added llvm_unreachable to address warning: this statement may fall through. NFC.

Galina Kistanova via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 12 12:28:50 PDT 2017


Thanks, David.

I share your doubt.

Unfortunately, the author is not around to ask what was intended/correct.
So, I want to have a trap there in case we will get a real case, then
somebody will complain and we will get more details.

I like the idea of having an assert instead of unreachable. That will serve
the same purpose but more cleanly. I will prepare a patch.

In any case I will follow up in a while to either address an issue the
asserts may introduce or to remove the redundancy from the code.

Thanks

Galina


On Mon, Jun 12, 2017 at 9:52 AM, David Blaikie <dblaikie at gmail.com> wrote:

> +1, this seems odd.
>
> If the code is really:
>
>   if (x)
>     y()
>   unreachable
>
> it should be replaced by:
>
>   assert(x);
>   y();
>
> but I /doubt/ that's what's intended/correct, though I could be wrong - I
> haven't looked at the code in any detail other than what's shown in the
> diff.
>
> On Sat, Jun 10, 2017 at 2:03 AM Davide Italiano via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> On Sat, Jun 10, 2017 at 1:04 AM, Galina Kistanova via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> > Author: gkistanova
>> > Date: Sat Jun 10 03:04:51 2017
>> > New Revision: 305144
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=305144&view=rev
>> > Log:
>> > Added llvm_unreachable to address warning: this statement may fall
>> through. NFC.
>> >
>>
>> Are you really sure `llvm_unreachable()` is the right choice here?
>> It seems a break would do it.
>>
>> --
>> Davide
>>
>> "There are no solved problems; there are only problems that are more
>> or less solved" -- Henri Poincare
>> _______________________________________________
>> 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/20170612/6ca45dbf/attachment.html>


More information about the llvm-commits mailing list