[PATCH vkd3d 4/6] vkd3d-shader: Implement hlsl_error() and hlsl_warning().
Matteo Bruni
matteo.mystral at gmail.com
Thu Feb 18 17:49:41 CST 2021
On Wed, Feb 17, 2021 at 6:52 AM Zebediah Figura <zfigura at codeweavers.com> wrote:
>
> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
> ---
> libs/vkd3d-shader/hlsl.c | 18 +-
> libs/vkd3d-shader/hlsl.h | 8 +-
> libs/vkd3d-shader/hlsl.l | 3 +-
> libs/vkd3d-shader/hlsl.y | 204 ++++++++++++-----------
> libs/vkd3d-shader/vkd3d_shader_private.h | 18 ++
> 5 files changed, 147 insertions(+), 104 deletions(-)
I generally like the changes to the error messages. Regardless, I have
a couple of comments...
> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> index c33d97c6..9ffabbcb 100644
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -768,12 +776,13 @@ static bool add_typedef(struct hlsl_ctx *ctx, DWORD modifiers, struct hlsl_type
>
> if ((type->modifiers & HLSL_MODIFIER_COLUMN_MAJOR)
> && (type->modifiers & HLSL_MODIFIER_ROW_MAJOR))
> - hlsl_error(ctx, v->loc, "more than one matrix majority keyword");
> + hlsl_error(ctx, v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
> + "'row_major' and 'column_major' modifiers are mutually exclusive.");
>
> ret = hlsl_scope_add_type(ctx->cur_scope, type);
> if (!ret)
> - hlsl_error(ctx, v->loc,
> - "redefinition of custom type '%s'", v->name);
> + hlsl_error(ctx, v->loc, VKD3D_SHADER_ERROR_HLSL_REDEFINED,
> + "Typedef '%s' is already defined.", v->name);
> vkd3d_free(v);
> }
> vkd3d_free(list);
This made me wonder whether defining a struct also implicitly defines
a type (i.e. can you reference a struct type by its name without
'struct' even when there is no explicit typedef?). A quick test
suggests that's the case, if that's confirmed we should probably fix
it at some point.
> @@ -1772,15 +1783,11 @@ struct_declaration:
> if (!$3)
> {
> if (!$2->name)
> - {
> - hlsl_error(ctx, @2,
> - "anonymous struct declaration with no variables");
> - }
> + hlsl_error(ctx, @2, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX,
> + "Struct type definitions cannot be anonymous.");
> if (modifiers)
> - {
> - hlsl_error(ctx, @1,
> - "modifier not allowed on struct type declaration");
> - }
> + hlsl_error(ctx, @1, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
> + "Modifiers are not allowed on struct type definitions.");
> }
>
> if (!(type = apply_type_modifiers(ctx, $2, &modifiers, @1)))
Splitting hairs, but I think "declaration" is a bit nicer for both of
the above (the C spec refers to those as declarations, fwiw). The
message could be improved to clarify that the declaration being
anonymous is only a problem if there are no variables being defined at
the same time (which I assume is what you mean with "Struct type
definitions"), but I don't have any good practical suggestions.
More information about the wine-devel
mailing list