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

[Galina Kistanova via llvm-commits]

I'm still not 100% certain those ifs are redundant and not intentional. But
if you are confident, that's fine.
Thanks for fixing this, David!

Thanks

Galina


On Mon, Jun 12, 2017 at 1:10 PM, David Blaikie <dblaikie at gmail.com> wrote:

> Looking further at this code, it looks like the 'if's could all be removed
> - there's the same condition ('if (Ty->isIntegerTy())') right before the
> switch... Removed all the noise in r305226
>
> On Mon, Jun 12, 2017 at 12:28 PM Galina Kistanova <gkistanova at gmail.com>
> wrote:
>
>> 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/c61e1b11/attachment.html>