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

[Zebediah Figura]

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.