poke-devel
[Top][All Lists]
Advanced

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

Re: [PATCH 3/3] pkl: Add array integrator


From: Jose E. Marchesi
Subject: Re: [PATCH 3/3] pkl: Add array integrator
Date: Sat, 08 Jan 2022 23:25:12 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux)

Hi Mohammad.

Awesome, thank you very much for doing this! :)

> +/* Return whether the type TYPE is integrable or not.  */
> +
> +int
> +pkl_ast_type_integrable_p (pkl_ast_node type)
> +{
> +  if (PKL_AST_TYPE_CODE (type) == PKL_TYPE_INTEGRAL)
> +    return 1;
> +  if (PKL_AST_TYPE_CODE (type) == PKL_TYPE_ARRAY)
> +    {
> +      pkl_ast_node etype = PKL_AST_TYPE_A_ETYPE (type);
> +
> +      switch (PKL_AST_TYPE_CODE (etype))
> +        {
> +        case PKL_TYPE_INTEGRAL:
> +          return 1;
> +        case PKL_TYPE_ARRAY:
> +        case PKL_TYPE_STRUCT:
> +          return pkl_ast_type_integrable_p (etype);
> +        default:
> +          return 0;
> +        }

Isn't this switch statement just replicating the logic of the function?
Why not:

if (PKL_AST_TYPE_CODE (type) == PKL_TYPE_ARRAY)
  return pkl_ast_type_integrable_p (etype);

> +    }
> +  /* Integral structs are integrable.  */
> +  if (PKL_AST_TYPE_CODE (type) == PKL_TYPE_STRUCT
> +      && PKL_AST_TYPE_S_ITYPE (type) != NULL)
> +    return 1;
> +  return 0;
> +}
> +
>  /* Build and return an expression that computes the size of TYPE in
>     bits, as an unsigned 64-bit value.  */

The rest of the patch looks supergood to me.
So OK for master.

PS: I am assuming you will document this in the manual in a subsequent
    patch ;)



reply via email to

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