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

[Zebediah Figura (she/her)]

On 12/11/20 12:11 PM, Henri Verbeet wrote:
> On Fri, 11 Dec 2020 at 00:27, Zebediah Figura <zfigura at 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?

Wrt the vkd3d_shader_message_context part—the current structure
definitely isn't a great fit for the preprocessor or HLSL parser (i.e.
the parts that use yacc) because the location tracking means that we'll
be setting a new location on every error call. (And of course most of
the other parsers we have don't really have a meaningful concept of
line/column locations...) But I wasn't sure enough to actually try to
redesign those parts, and it seemed like something that could be
deferred, so I just left it alone...

Wrt preproc_location vs preproc_ctx—part of the rub here is that {line,
column} is a bit different from {first_line, first_column}; i.e. if we
were to add {last_line, last_column}, that wouldn't be a part of
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)
> 

Excellent point, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0x0D9D358A07A17840.asc
Type: application/pgp-keys
Size: 1769 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201211/49c577b2/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201211/49c577b2/attachment.sig>