[Mesa-dev] [PATCH] glsl: remove unreachable assert()

[Emil Velikov]

On 10 April 2018 at 17:53, Ivan Kalvachev <ikalvachev at gmail.com> wrote:
> On 3/28/18, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> Earlier commit enforced that we'll bail out if the number of terminators
>> is different than 2. With that in mind, the assert() will never trigger.
>>
>> Fixes: 56b867395de ("glsl: fix infinite loop caused by bug in loop
>> unrolling pass")
>> Cc: Timothy Arceri <tarceri at itsqueeze.com>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>
> Just a nitpick.
> The explanations doesn't sound right to me.
>
> Asserts are meant to never trigger.
> They are used to check the internal logic of the code.
>
> If this assert does trigger that would mean
> that there is a bug in the code that makes sure
> the number of terminators is different than 2.
>
> It is better to catch bug with assert than
> to silently do something wrong.
>
Right. wording is not perfect. As-is the assert provides misleading
assumption considering the explicit check.

> Also, sometimes compilers might use
> the assert assumptions to optimize the code.
> (Even when the assertion itself is disabled.)
>
Fully aware of that, yet i doubt it will matter in this case.
If you want to give it a check, that would be appreciated.

JFYI I've pushed this ~2h before your reply. But if people prefer I
can revert it.

Thanks
Emil