Re: [PATCH v2 vkd3d 1/4] vkd3d-shader: Handle preprocessor parsing errors.

[Henri Verbeet]

On Fri, 11 Dec 2020 at 00:27, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
> +struct preproc_location
> +{
> +    const char *filename;
> +    unsigned int first_line, first_column;
> +};
> +
>  struct preproc_ctx
>  {
>      void *scanner;
>
> +    struct vkd3d_shader_message_context *message_context;
>      struct vkd3d_string_buffer buffer;
> +    unsigned int line, column;
> +    const char *source_name;
> +
> +    bool error;
>  };

There seems to be some duplication of location information between
struct vkd3d_shader_message_context, struct preproc_ctx, and struct
preproc_location. Is that intentional? If it is, would it make sense
to introduce e.g. a struct vkd3d_shader_source_location instead of
struct preproc_location, and then use that structure in both struct
vkd3d_shader_message_context and struct preproc_ctx?
> +#define YYLLOC_DEFAULT(cur, rhs, n) \
> +    do                                                          \
> +    {                                                           \
> +        if (n)                                                  \
> +        {                                                       \
> +            (cur).filename = YYRHSLOC(rhs, 1).filename;         \
> +            (cur).first_line = YYRHSLOC(rhs, 1).first_line;     \
> +            (cur).first_column = YYRHSLOC(rhs, 1).first_column; \
> +        }                                                       \
> +        else                                                    \
> +        {                                                       \
> +            (cur).filename = YYRHSLOC(rhs, 0).filename;         \
> +            (cur).first_line = YYRHSLOC(rhs, 0).first_line;     \
> +            (cur).first_column = YYRHSLOC(rhs, 0).first_column; \
> +        }                                                       \
> +    } while (0)
> +
> Generally speaking, we would try to avoid multi-line macros by doing
> something like the following where possible:

static inline void preproc_yylloc_default(struct preproc_location
*cur, const struct preproc_location *rhs, int n)
    {
        if (n)
        {
            cur->filename = YYRHSLOC(rhs, 1).filename;
            cur->first_line = YYRHSLOC(rhs, 1).first_line;
            cur->first_column = YYRHSLOC(rhs, 1).first_column;
        }
        else
        {
            cur->filename = YYRHSLOC(rhs, 0).filename;
            cur->first_line = YYRHSLOC(rhs, 0).first_line;
            cur->first_column = YYRHSLOC(rhs, 0).first_column;
        }
    }

    #define YYLLOC_DEFAULT(cur, rhs, n) preproc_yylloc_default(&cur, rhs, n)

In this particular case though, I think all that essentially reduces to:

    #define YYLLOC_DEFAULT(cur, rhs, n) (cur) = YYRHSLOC(rhs, !!n)