Re: [PATCH] pickles: Use EXCOND operator in {b,c}tf-dump.pk

[David Faust]

Hi Mohammad-Reza,

On 12/26/21 16:56, Mohammad-Reza Nabipoor wrote:
> 2021-12-27  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>
>         * pickles/btf-dump.pk (btf_dump_type_vdata): Use EXCOND operator.
>         * pickles/ctf-dump.pk (ctf_dump_vlen_data): Likewise.
> ---
>
> Hi, David and Indu.
>
> I didn't actually "run" this code, because I don't know much about BTF/CTF.
> Could you please verify that this patch actually works?
> (And it'd be nice if we have some tests for these pickles in
> `testsuite/poke.pickles/`).

This is very nice, certainly much cleaner. Thanks for doing it :)

I tested both CTF and BTF (BTF more extensively since I have more laying 
around).

It works great with slight modification - since the individual dump 
functions are void, the calls need to be turned into statements by 
wrapping with { }, or else we fail with

  error: function doesn't return a value

i.e.
+    ({ btf_dump_int (t.data.integer); } ?! E_elem)
+      || ({ btf_dump_array (t.data.array); } ?! E_elem)
... etc.

Likewise for CTF. (Attached the edited hunks for convenience, please 
use/modify/ignore them as you like :) )

Otherwise LGTM, and thanks for the reminder of tests I will look into 
adding them.

> Thanks,
> Mohammad-Reza
>
>
>   ChangeLog           |  5 ++++
>   pickles/btf-dump.pk | 61 ++++++---------------------------------------
>   pickles/ctf-dump.pk | 25 ++++++-------------
>   3 files changed, 19 insertions(+), 72 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index e00fa631..667e2b8c 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2021-12-27  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> +
> +       * pickles/btf-dump.pk (btf_dump_type_vdata): Use EXCOND operator.
> +       * pickles/ctf-dump.pk (ctf_dump_vlen_data): Likewise.
> +
>   2021-12-27  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>   
>   	* libpoke/pvm.jitter (state-struct-backing-c): Remove `canary` field.
> diff --git a/pickles/btf-dump.pk b/pickles/btf-dump.pk
> index 26ea98f0..8aeb5069 100644
> --- a/pickles/btf-dump.pk
> +++ b/pickles/btf-dump.pk
> @@ -99,60 +99,13 @@ fun btf_dump_datasec = (BTF_Var_SecInfo[] entries) void:
>   
>   fun btf_dump_type_vdata = (BTF_Section btf, BTF_Type t) void:
>     {
> -    /* Integer */
> -    try
> -    {
> -      btf_dump_int (t.data.integer);
> -      return;
> -    }
> -    catch if E_elem {}
> -
> -    /* Array */
> -    try
> -    {
> -      btf_dump_array (t.data.array);
> -      return;
> -    }
> -    catch if E_elem {}
> -
> -    /* Enum */
> -    try
> -    {
> -      btf_dump_enum (btf, t.data.enum);
> -      return;
> -    }
> -    catch if E_elem {}
> -
> -    /* Func proto */
> -    try
> -    {
> -      btf_dump_proto (btf, t.data.func_proto.params);
> -      return;
> -    }
> -    catch if E_elem {}
> -
> -    /* Variable.  */
> -    try
> -    {
> -      btf_dump_var (t.data.variable);
> -      return;
> -    }
> -    catch if E_elem {}
> -
> -    /* Struct or union.  */
> -    try
> -    {
> -      btf_dump_sou (btf, t);
> -      return;
> -    }
> -    catch if E_elem {}
> -
> -    /* Datasec */
> -    try
> -    {
> -      btf_dump_datasec (t.data.datasec);
> -    }
> -    catch if E_elem {}
> +    (btf_dump_int (t.data.integer) ?! E_elem)
> +      || (btf_dump_array (t.data.array) ?! E_elem)
> +      || (btf_dump_enum (btf, t.data.enum) ?! E_elem)
> +      || (btf_dump_proto (btf, t.data.func_proto.params) ?! E_elem)
> +      || (btf_dump_var (t.data.variable) ?! E_elem)
> +      || (btf_dump_sou (btf, t) ?! E_elem)
> +      || (btf_dump_datasec (t.data.datasec) ?! E_elem);
>     }
>   
>   /* Dump a complete BTF type. Includes lookup and display of any strings
> diff --git a/pickles/ctf-dump.pk b/pickles/ctf-dump.pk
> index 52a42480..82fb54d4 100644
> --- a/pickles/ctf-dump.pk
> +++ b/pickles/ctf-dump.pk
> @@ -116,24 +116,13 @@ fun ctf_dump_func = (CTF_Dictionary ctf, CTF_Type t) void:
>   
>   fun ctf_dump_vlen_data = (CTF_Dictionary ctf, CTF_Type t) void:
>   {
> -  try ctf_dump_int (t.data.integer);
> -  catch if E_elem {};
> -
> -  try ctf_dump_array (t.data.array);
> -  catch if E_elem {};
> -
> -  try ctf_dump_slice (ctf, t.data.slice);
> -  catch if E_elem {};
> -
> -  try ctf_dump_sou_members (ctf, t.data.members);
> -  catch if E_elem {};
> -
> -  try ctf_dump_enum (ctf, t.data.enum);
> -  catch if E_elem {};
> -
> -  /* XXX CTF_Func_Args declaration scoped within CTF_Type is not available.  */
> -  try ctf_dump_func (ctf, t);
> -  catch if E_elem {};
> +  (ctf_dump_int (t.data.integer) ?! E_elem)
> +    || (ctf_dump_array (t.data.array) ?! E_elem)
> +    || (ctf_dump_slice (ctf, t.data.slice) ?! E_elem)
> +    || (ctf_dump_sou_members (ctf, t.data.members) ?! E_elem)
> +    || (ctf_dump_enum (ctf, t.data.enum) ?! E_elem)
> +    /* XXX CTF_Func_Args declaration scoped within CTF_Type is not available.  
> */
> +    || (ctf_dump_func (ctf, t) ?! E_elem);
>   }
>   
>   /* Dump the given CTF type.  */

pickles-use-EXCOND-operator_edit.patch
Description: Text Data