From: "Zebediah Figura (she/her)" Subject: Re: [PATCH vkd3d] vkd3d-shader/hlsl: Do not swallow compilation errors. Message-Id: <5fccf36f-a0e9-a088-43bd-bec5211ee5a3@codeweavers.com> Date: Fri, 27 Aug 2021 12:18:07 -0500 In-Reply-To: <20210827101259.3760205-1-gmascellani@codeweavers.com> References: <20210827101259.3760205-1-gmascellani@codeweavers.com> On 8/27/21 5:12 AM, Giovanni Mascellani wrote: > There are cases where compilation fails, but no error message is > recorded. They usually correspond to unimplemented features, but > still failure should be forwarded to the caller, otherwise it will > manifest itself later in more confusing ways (like not being able > to find a function even though it was apparently correctly compiled). > > Signed-off-by: Giovanni Mascellani > --- > libs/vkd3d-shader/hlsl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c > index 0d9186ae..aafd8acb 100644 > --- a/libs/vkd3d-shader/hlsl.c > +++ b/libs/vkd3d-shader/hlsl.c > @@ -1803,10 +1803,10 @@ int hlsl_compile_shader(const struct vkd3d_shader_code *hlsl, const struct vkd3d > if (!hlsl_ctx_init(&ctx, profile, message_context)) > return VKD3D_ERROR_OUT_OF_MEMORY; > > - if (hlsl_lexer_compile(&ctx, hlsl) == 2) > + if ((ret = hlsl_lexer_compile(&ctx, hlsl))) > { > hlsl_ctx_cleanup(&ctx); > - return VKD3D_ERROR_OUT_OF_MEMORY; > + return ret == 2 ? VKD3D_ERROR_OUT_OF_MEMORY : VKD3D_ERROR_INVALID_SHADER; > } > > if (ctx.result) > We should really be using hlsl_fixme() for such cases (or, in some cases, not aborting at all). The patch seems like a good idea regardless, though. That said, I think we should preserve an existing ctx->result if it's nonzero. In particular, allocation failure will (and should) print YYABORT, which causes yyparse() to return 1, not 2, but we do want to return VKD3D_ERROR_OUT_OF_MEMORY there.