Re: [PATCH vkd3d 1/5] vkd3d-shader: Partially implement #define.

[Matteo Bruni]

On Tue, Jan 5, 2021 at 5:17 PM Zebediah Figura  wrote:
> Signed-off-by: Zebediah Figura
>
> Makefile.am                              |  1 +
>  libs/vkd3d-shader/preproc.h              |  9 ++++
>  libs/vkd3d-shader/preproc.l              | 24 ++++++++--
>  libs/vkd3d-shader/preproc.y              | 60 +++++++++++++++++++++++-
>  libs/vkd3d-shader/vkd3d_shader_private.h |  1 +
>  tests/hlsl_d3d12.c                       |  2 +-
>  6 files changed, 92 insertions(+), 5 deletions(-)
>
> diff --git a/libs/vkd3d-shader/preproc.h b/libs/vkd3d-shader/preproc.h
> index effcd995..1cb3d5d7 100644
> --- a/libs/vkd3d-shader/preproc.h
> +++ b/libs/vkd3d-shader/preproc.h
> @@ -23,6 +23,12 @@
>
> #include "vkd3d_shader_private.h"
>
> +struct preproc_macro
> +{
> - struct list entry;
> - char *name;
>
> +};
> -
>
> struct preproc_if_state
>  {
>      /* Are we currently in a "true" block? */
> @@ -41,6 +47,8 @@ struct preproc_ctx
>      struct vkd3d_string_buffer buffer;
>      struct vkd3d_shader_location location;
> - struct list macros;
> - struct preproc_if_state *if_stack;
>    size_t if_count, if_stack_size;
>
> diff --git a/libs/vkd3d-shader/preproc.y b/libs/vkd3d-shader/preproc.y
> index 647bb45d..e608c5d8 100644
> --- a/libs/vkd3d-shader/preproc.y
> +++ b/libs/vkd3d-shader/preproc.y
> @@ -66,6 +66,44 @@ static void yyerror(const YYLTYPE *loc, void *scanner, struct preproc_ctx *ctx,
>      preproc_error(ctx, loc, VKD3D_SHADER_ERROR_PP_INVALID_SYNTAX, "%s", string);
>  }
>
> +static struct preproc_macro *preproc_find_macro(struct preproc_ctx *ctx, const char *name)
> +{
> - struct preproc_macro *macro;
> -
> - LIST_FOR_EACH_ENTRY(macro, &ctx->macros, struct preproc_macro, entry)
> - {
> -
>    if (!strcmp(macro->name, name))
> -
>        return macro;
> - }
> - return NULL;
>
> +}
> -
>
> +static bool preproc_add_macro(struct preproc_ctx *ctx, const struct vkd3d_shader_location *loc, char *name)
> +{
> - struct preproc_macro *macro;
> -
> - if ((macro = preproc_find_macro(ctx, name)))
> - {
> -
>    preproc_warning(ctx, loc, VKD3D_SHADER_WARNING_PP_ALREADY_DEFINED, "Redefinition of %s.", name);
> -
>    preproc_free_macro(macro);
> - }
> -
> - TRACE("Defining new macro %s.\n", debugstr_a(name));
> -
> - if (!(macro = vkd3d_malloc(sizeof(*macro))))
> -
>    return false;
> - macro->name = name;
> - list_add_tail(&ctx->macros, &macro->entry);
> - return true;
>
> +}

Not a big deal right now that defines are only looked up when
searching for duplicates, and possibly neither in the future, but I'm
a bit uncomfortable with using an unsorted list for defines. Maybe
HLSL shaders in the wild are generally low on #define usage, but it
seems to me that it isn't much more complicated to use e.g. an rbtree
instead (to name another data structure which is readily available in
vkd3d) and I can't think of other obvious downsides to taking care of
this right away.