poke-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] pkl: fix invalid access to STRUCT_TYPE_FIELD node


From: Jose E. Marchesi
Subject: Re: [PATCH] pkl: fix invalid access to STRUCT_TYPE_FIELD node
Date: Thu, 06 Jul 2023 10:15:03 +0200
User-agent: Gnus/5.13 (Gnus v5.13)

Hi Mohammad.
Thanks for the patch.

> Using PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P accessor on a PKL_AST_DECL
> node happend to not cause any problem till the previous commit, in which,
> we removed a field and added two new fields.  That change caused
> PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P to return a non-zero value on 32-bit
> platforms (i386 and armv7).  The result was a messed up lexical environment.
>
> 2023-07-06  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>
>       * libpoke/pkl-gen.pks (struct_mapper): Make sure `@field' is of
>       type `PKL_AST_STRUCT_TYPE_FIELD' before using the field accessor.
>       (struct_constructor): Likewise.
> ---
>  ChangeLog           |  6 ++++++
>  libpoke/pkl-gen.pks | 15 +++++++++++----
>  2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index eb347477..200db0a9 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2023-07-06  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> +
> +     * libpoke/pkl-gen.pks (struct_mapper): Make sure `@field' is of
> +     type `PKL_AST_STRUCT_TYPE_FIELD' before using the field accessor.
> +     (struct_constructor): Likewise.
> +
>  2023-07-02  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>  
>       * libpoke/pkl-ast.h (PKL_AST_DECL_PREV_DECL): New macro.
> diff --git a/libpoke/pkl-gen.pks b/libpoke/pkl-gen.pks
> index 0faa98c9..09177156 100644
> --- a/libpoke/pkl-gen.pks
> +++ b/libpoke/pkl-gen.pks
> @@ -1393,7 +1393,10 @@
>   .c     if (PKL_AST_CODE (@field) != PKL_AST_DECL
>   .c         || PKL_AST_DECL_KIND (@field) != PKL_AST_DECL_KIND_TYPE)
>   .c     {
> - .c       if (!PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
> + .c       if (PKL_AST_CODE (@field) == PKL_AST_STRUCT_TYPE_FIELD
> + .c           && PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
> + .c         ;
> + .c       else
>   .c         i++;

I would find it more clear to use positive logic in the conditional:

if (PKL_AST_CODE (@field) != PKL_AST_STRUCT_TYPE_FIELD
    || !PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
  i++;

>   .c     }
>   .c     continue;
> @@ -1698,10 +1701,9 @@
>          .label .omitted_field
>          .label .got_value
>          .label .constructed_value
> -        .let @field_initializer = PKL_AST_STRUCT_TYPE_FIELD_INITIALIZER 
> (@field)
> -        .let @field_type = PKL_AST_STRUCT_TYPE_FIELD_TYPE (@field)
>   .c   if (PKL_AST_CODE (@field) != PKL_AST_STRUCT_TYPE_FIELD)
>   .c   {
> + .c     assert (PKL_AST_CODE (@field) == PKL_AST_DECL);
>   .c     /* This is a declaration.  Generate it.  */
>   .c     PKL_GEN_PUSH_CONTEXT;
>   .c     PKL_PASS_SUBPASS (@field);
> @@ -1713,6 +1715,8 @@
>   .c
>   .c     continue;
>   .c   }
> +        .let @field_initializer = PKL_AST_STRUCT_TYPE_FIELD_INITIALIZER 
> (@field)
> +        .let @field_type = PKL_AST_STRUCT_TYPE_FIELD_TYPE (@field)
>   .c     /* If this is a computed field, ignore it.  */
>   .c   if (PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
>   .c     continue;
> @@ -2026,7 +2030,10 @@
>   .c     if (PKL_AST_CODE (@field) != PKL_AST_DECL
>   .c         || PKL_AST_DECL_KIND (@field) != PKL_AST_DECL_KIND_TYPE)
>   .c     {
> - .c       if (!PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
> + .c       if (PKL_AST_CODE (@field) == PKL_AST_STRUCT_TYPE_FIELD
> + .c           && PKL_AST_STRUCT_TYPE_FIELD_COMPUTED_P (@field))
> + .c         ;
> + .c       else

Ditto.

>   .c         i++;
>   .c     }
>   .c     continue;

Given that, LGTM.
OK for both master and maint/poke-3.

Thanks!



reply via email to

[Prev in Thread] Current Thread [Next in Thread]