[PATCH vkd3d] vkd3d: Add a function to mark unreachable code.

[Gabriel Ivăncescu]

On 14/04/2022 19:14, Zebediah Figura wrote:
> On 4/14/22 06:00, Giovanni Mascellani wrote:
>> Hi,
>>
>> Il 08/04/22 19:00, Zebediah Figura ha scritto:
>>> I don't think there's value in specifying a message for cases like 
>>> this, and I'd rather not have to do so in new code. This applies to 
>>> pretty much anywhere we have an assert() in a switch default case.
>>>
>>> (Ideally we should avoid the 'default' keyword where possible, 
>>> although in some cases this is difficult.)
>>
>> vkd3d_unreachable() is written in a way such that you can pass it NULL 
>> to it for precisely this reason. I went adding messages for all 
>> present uses, but it is not mandatory to have one. Does this address 
>> your objection?
> 
> I noticed that, but even so, I don't think there's value in printing a 
> message in any of these cases. A message only makes sense when we expect 
> it to be printed, and we don't expect any of these messages to ever be 
> printed. The only reason I added assert(0) was for code clarity, and I 
> think it's quite clear why the default code paths are unreachable.
> 
>>
>>>> @@ -1542,12 +1534,10 @@ static void write_sm4_cast(struct hlsl_ctx 
>>>> *ctx,
>>>>               break;
>>>>           case HLSL_TYPE_BOOL:
>>>> -            /* Casts to bool should have already been lowered. */
>>>> -            assert(0);
>>>> -            break;
>>>> +            vkd3d_unreachable("Casts to bool should have already 
>>>> been lowered");
>>>
>>> This one I think should remain an assert. I can more easily see this 
>>> getting triggered due to some forgotten implicit agreement. Hence we 
>>> probably shouldn't tell the compiler this code path is unreachable.
>>>
>>> Using an assert(!"string") or assert(0 && "string") construction (or 
>>> defining our own macro) seems reasonable to me, though.
>>
>>  From the point of view of the compiler, assert(0) is not really 
>> different from vkd3d_unreachable(NULL): nothing tells the compiler 
>> that control will never ever reach it; if it happens, the compiler 
>> will print a message and abort(). The advantage of vkd3d_unreachable() 
>> is that the compiler won't try to tell you "Hey, you didn't write your 
>> return statement!" in places where it doesn't make any sense (because 
>> it's marked "noreturn").
> 
> As it's currently written, yes. However, I think we want to hook this up 
> to __builtin_unreachable() if the latter is available.
> 

What about something like DEFAULT_UNREACHABLE from winnt.h?