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?
@@ -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").
Thanks, Giovanni.