Re: [PATCH] pkl,doc,testsuite,pickles: Invert the value of EXCOND ?! operator

[Jose E. Marchesi]

Hi Mohammad.

The patch is OK for master.
Thanks!

> The current value of EXCOND ?! operator is not intuitive. This commit
> changes the semantics to evaluates to 1 (true) if the execution of the
> first operand raises the specified exception, and 0 (false) otherwise.
>
> The following example will print the message on the terminal:
>
>   var zero = 0;
>   if (1/zero ?! E_div_by_zero)
>     print "division by zero happened!\n";
>
> 2022-07-09  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>
>       * libpoke/pkl-gen.c (pkl_gen_pr_op_excond): Invert the logic of excond
>       operator.
>       * doc/poke.texi (exception predicate): Update to new semantics.
>       * libpoke/std.pk (with_temp_ios): Likewise.
>       libpoke/std.pk (with_cur_ios): Likewise.
>       * pickles/asn1-ber.pk (BER_Data_Value.isdefinite): Likewise.
>       * pickles/btf-dump.pk (btf_dump_type_vdata): Likewise.
>       * pickles/ctf-dump.pk (ctf_dump_func): Likewise.
>       * pickles/diff.pk (diff_structured): Likewise.
>       * testsuite/poke.pkl/excond-1.pk: Likewise.
>       * testsuite/poke.pkl/excond-2.pk: Likewise.
>       * testsuite/poke.pkl/excond-3.pk: Likewise.
>       * testsuite/poke.pkl/excond-4.pk: Likewise.
>       * testsuite/poke.std/std-test.pk: Likewise.
> ---
>  ChangeLog                      | 17 +++++++++++++++++
>  doc/poke.texi                  |  8 ++++----
>  libpoke/pkl-gen.c              |  8 ++++----
>  libpoke/std.pk                 |  4 ++--
>  pickles/asn1-ber.pk            |  2 +-
>  pickles/btf-dump.pk            | 12 ++++++------
>  pickles/ctf-dump.pk            | 10 +++++-----
>  pickles/diff.pk                | 18 +++++++++---------
>  testsuite/poke.pkl/excond-1.pk |  2 +-
>  testsuite/poke.pkl/excond-2.pk |  2 +-
>  testsuite/poke.pkl/excond-3.pk |  2 +-
>  testsuite/poke.pkl/excond-4.pk |  2 +-
>  testsuite/poke.std/std-test.pk |  4 ++--
>  13 files changed, 54 insertions(+), 37 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 766819a5..fa1da0d7 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,20 @@
> +2022-07-09  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> +
> +     * libpoke/pkl-gen.c (pkl_gen_pr_op_excond): Invert the logic of excond
> +     operator.
> +     * doc/poke.texi (exception predicate): Update to new semantics.
> +     * libpoke/std.pk (with_temp_ios): Likewise.
> +     libpoke/std.pk (with_cur_ios): Likewise.
> +     * pickles/asn1-ber.pk (BER_Data_Value.isdefinite): Likewise.
> +     * pickles/btf-dump.pk (btf_dump_type_vdata): Likewise.
> +     * pickles/ctf-dump.pk (ctf_dump_func): Likewise.
> +     * pickles/diff.pk (diff_structured): Likewise.
> +     * testsuite/poke.pkl/excond-1.pk: Likewise.
> +     * testsuite/poke.pkl/excond-2.pk: Likewise.
> +     * testsuite/poke.pkl/excond-3.pk: Likewise.
> +     * testsuite/poke.pkl/excond-4.pk: Likewise.
> +     * testsuite/poke.std/std-test.pk: Likewise.
> +
>  2022-07-08  Jose E. Marchesi  <jose.marchesi@oracle.com>
>  
>       * pickles/btf.pk (BTF_Int): Turn `encoding' into a nested integral
> diff --git a/doc/poke.texi b/doc/poke.texi
> index 9943324f..d18299bd 100644
> --- a/doc/poke.texi
> +++ b/doc/poke.texi
> @@ -13667,7 +13667,7 @@ the @dfn{exception predicate} operator, @code{?!}, 
> that has two forms:
>  
>  In the first form, the expression @var{exp} is executed.  If the
>  execution raises the exception @var{exception} then the result of the
> -predicate is @code{0} (false), otherwise it is @code{1} (true).  The
> +predicate is @code{1} (true), otherwise it is @code{0} (false).  The
>  value of the expression is discarded.
>  
>  In the second form, the compound statement passed as the first operand
> @@ -13682,9 +13682,9 @@ a much more readable way:
>  method _print = void:
>  @{
>    if (simple_value ?! E_elem)
> -    print "#<%u32d>", simple_value;
> -  else
>      print "%v\n", complex_value;
> +  else
> +    print "#<%u32d>", simple_value;
>  @}
>  @end example
>  
> @@ -13695,7 +13695,7 @@ Or, alternatively (and slightly more efficiently):
>  method _print = void:
>  @{
>    (@{ print "<%u32d>", simple_value; @} ?! E_elem)
> -    || print "%v\n", complex_value;
> +    && print "%v\n", complex_value;
>  @}
>  @end example
>  
> diff --git a/libpoke/pkl-gen.c b/libpoke/pkl-gen.c
> index af26f059..d0d6138e 100644
> --- a/libpoke/pkl-gen.c
> +++ b/libpoke/pkl-gen.c
> @@ -4254,8 +4254,8 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_op_excond)
>    pvm_program_label done = pkl_asm_fresh_label (PKL_GEN_ASM);
>  
>    /* Push the provisional result of the operation, which is
> -     `true'.  */
> -  pkl_asm_insn (pasm, PKL_INSN_PUSH, pvm_make_int (1, 32));
> +     `false'.  */
> +  pkl_asm_insn (pasm, PKL_INSN_PUSH, pvm_make_int (0, 32));
>  
>    /* Install a handler for the exception specified in the second
>       operand.  */
> @@ -4272,12 +4272,12 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_op_excond)
>    pkl_asm_insn (pasm, PKL_INSN_BA, done);
>  
>    /* The exception handler just drops the raised exception and the
> -     provisional result `true' and pushes `false' to reflect the
> +     provisional result `false' and pushes `true' to reflect the
>       exception was raised. */
>    pkl_asm_label (pasm, exception_handler);
>    pkl_asm_insn (pasm, PKL_INSN_DROP); /* The exception.  */
>    pkl_asm_insn (pasm, PKL_INSN_DROP); /* The provisional result.  */
> -  pkl_asm_insn (pasm, PKL_INSN_PUSH, pvm_make_int (0, 32));
> +  pkl_asm_insn (pasm, PKL_INSN_PUSH, pvm_make_int (1, 32));
>  
>    pkl_asm_label (pasm, done);
>    PKL_PASS_BREAK;
> diff --git a/libpoke/std.pk b/libpoke/std.pk
> index e7cb8ef0..2b948f95 100644
> --- a/libpoke/std.pk
> +++ b/libpoke/std.pk
> @@ -441,7 +441,7 @@ fun with_temp_ios = (string handler = 
> pk_get_unique_mem_ios_handler,
>                       Pk_With_Ios_Fn do = lambda void:{},
>                       int<32> endian = get_endian) void:
>  {
> -  var old_ios = get_ios ?! E_no_ios ? get_ios : -1;
> +  var old_ios = get_ios ?! E_no_ios ? -1 : get_ios;
>    var old_endian = get_endian;
>    var new_ios = open (handler, flags);
>  
> @@ -474,7 +474,7 @@ fun with_cur_ios = (int<32> ios,
>                      Pk_With_Ios_Fn do = lambda void:{},
>                      int<32> endian = get_endian) void:
>  {
> -  var old_ios = get_ios ?! E_no_ios ? get_ios : -1;
> +  var old_ios = get_ios ?! E_no_ios ? -1 : get_ios;
>    var old_endian = get_endian;
>  
>    set_ios (ios);
> diff --git a/pickles/asn1-ber.pk b/pickles/asn1-ber.pk
> index 8f9c2deb..af185c9d 100644
> --- a/pickles/asn1-ber.pk
> +++ b/pickles/asn1-ber.pk
> @@ -217,7 +217,7 @@ type BER_Data_Value =
>  
>        method is_indefinite = int:
>        {
> -        return indefinite ?! E_elem;
> +        return !(indefinite ?! E_elem);
>        }
>  
>        method get = offset<uint<64>,B>:
> diff --git a/pickles/btf-dump.pk b/pickles/btf-dump.pk
> index 10e44941..f18479d0 100644
> --- a/pickles/btf-dump.pk
> +++ b/pickles/btf-dump.pk
> @@ -117,12 +117,12 @@ fun btf_dump_type_vdata = (BTF_Section btf, BTF_Type t) 
> void:
>      }
>  
>      ({ 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);
> +      && ({ 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 7bb1f9fd..edf30dab 100644
> --- a/pickles/ctf-dump.pk
> +++ b/pickles/ctf-dump.pk
> @@ -117,12 +117,12 @@ fun ctf_dump_func = (CTF_Dictionary ctf, CTF_Type t) 
> void:
>  fun ctf_dump_vlen_data = (CTF_Dictionary ctf, CTF_Type t) void:
>  {
>    ({ 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)
> +    && ({ 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);
> +    && ({ ctf_dump_func (ctf, t); } ?! E_elem);
>  }
>  
>  /* Dump the given CTF type.  */
> diff --git a/pickles/diff.pk b/pickles/diff.pk
> index 38d1c77b..9ca563c0 100644
> --- a/pickles/diff.pk
> +++ b/pickles/diff.pk
> @@ -251,15 +251,15 @@ fun diff_structured = (any a, any b,
>    {
>      for (; idx < val'length; ++idx)
>      {
> -      if (val'elem (idx) ?! E_inval)
> +      if (!(val'elem (idx) ?! E_inval))
>        {
>          /* Note that recursing into the element may result in E_inval
>             in case a non-byte element is found.  In that case, try to
>             emit the thunk anyway.  */
>          if (elem_simple_p (val, idx)
> -            || ! { sdiff_addrem (what, val'elem (idx),
> -                                 prefix + format_ename (val'ename (idx)),
> -                                 0); } ?! E_inval)
> +            || { sdiff_addrem (what, val'elem (idx),
> +                               prefix + format_ename (val'ename (idx)),
> +                               0); } ?! E_inval)
>            thunk.addrem :what what :off val'eoffset (idx) :siz val'esize (idx)
>                         :bytes get_elem_bytes (val, idx)
>                         :name prefix + format_ename (val'ename (idx))
> @@ -275,7 +275,7 @@ fun diff_structured = (any a, any b,
>      var idx = 0UL;
>      for (; idx < a'length; ++idx)
>      {
> -      var a_is_present = a'elem (idx) ?! E_inval;
> +      var a_is_present = !(a'elem (idx) ?! E_inval);
>  
>        if (idx >= b'length)
>        {
> @@ -284,7 +284,7 @@ fun diff_structured = (any a, any b,
>        }
>        else
>        {
> -        var b_is_present = b'elem (idx) ?! E_inval;
> +        var b_is_present = !(b'elem (idx) ?! E_inval);
>  
>          if (a_is_present && b_is_present)
>          {
> @@ -297,8 +297,8 @@ fun diff_structured = (any a, any b,
>               non-byte element is found.  In that case, try to emit the
>               thunk anyway.  */
>            if (elem_simple_p (a, idx)
> -              || ! { sdiff_change (a'elem (idx), b'elem (idx),
> -                                   a_full_name, b_full_name); } ?! E_inval)
> +              || { sdiff_change (a'elem (idx), b'elem (idx),
> +                                 a_full_name, b_full_name); } ?! E_inval)
>            {
>              var a_bytes = get_elem_bytes (a, idx);
>              var b_bytes = get_elem_bytes (b, idx);
> @@ -330,7 +330,7 @@ fun diff_structured = (any a, any b,
>  
>    try
>    {
> -    if (! { sdiff_change (a, b, prefix_a, prefix_b); } ?! E_inval)
> +    if ({ sdiff_change (a, b, prefix_a, prefix_b); } ?! E_inval)
>      {
>        var a_bytes = get_bytes (a);
>        var b_bytes = get_bytes (b);
> diff --git a/testsuite/poke.pkl/excond-1.pk b/testsuite/poke.pkl/excond-1.pk
> index dea92b0d..94bcf0d0 100644
> --- a/testsuite/poke.pkl/excond-1.pk
> +++ b/testsuite/poke.pkl/excond-1.pk
> @@ -1,4 +1,4 @@
>  /* { dg-do run } */
>  
>  /* { dg-command { 2 ?! E_generic } } */
> -/* { dg-output "1" } */
> +/* { dg-output "0" } */
> diff --git a/testsuite/poke.pkl/excond-2.pk b/testsuite/poke.pkl/excond-2.pk
> index 662140d4..b716162b 100644
> --- a/testsuite/poke.pkl/excond-2.pk
> +++ b/testsuite/poke.pkl/excond-2.pk
> @@ -3,4 +3,4 @@
>  var zero = 0;
>  
>  /* { dg-command { 2/zero ?! E_div_by_zero } } */
> -/* { dg-output "0" } */
> +/* { dg-output "1" } */
> diff --git a/testsuite/poke.pkl/excond-3.pk b/testsuite/poke.pkl/excond-3.pk
> index 7289f5e8..adb27aaf 100644
> --- a/testsuite/poke.pkl/excond-3.pk
> +++ b/testsuite/poke.pkl/excond-3.pk
> @@ -1,4 +1,4 @@
>  /* { dg-do run } */
>  
>  /* { dg-command { { print "lala\n"; raise E_generic; } ?! E_generic } } */
> -/* { dg-output "lala\n0" } */
> +/* { dg-output "lala\n1" } */
> diff --git a/testsuite/poke.pkl/excond-4.pk b/testsuite/poke.pkl/excond-4.pk
> index 7289f5e8..adb27aaf 100644
> --- a/testsuite/poke.pkl/excond-4.pk
> +++ b/testsuite/poke.pkl/excond-4.pk
> @@ -1,4 +1,4 @@
>  /* { dg-do run } */
>  
>  /* { dg-command { { print "lala\n"; raise E_generic; } ?! E_generic } } */
> -/* { dg-output "lala\n0" } */
> +/* { dg-output "lala\n1" } */
> diff --git a/testsuite/poke.std/std-test.pk b/testsuite/poke.std/std-test.pk
> index 1a45fd6a..03f1013d 100644
> --- a/testsuite/poke.std/std-test.pk
> +++ b/testsuite/poke.std/std-test.pk
> @@ -210,12 +210,12 @@ var tests = [
>          with_temp_ios :handler "*bar*"
>                        :endian !end
>                        :do lambda void: { a = int<32> @ 0#B; };
> -        assert (!(get_ios ?! E_no_ios));
> +        assert (get_ios ?! E_no_ios);
>          assert (a == 0);
>  
>          /* No arguments.  */
>          with_temp_ios;
> -        assert (!(get_ios ?! E_no_ios));
> +        assert (get_ios ?! E_no_ios);
>  
>          /* Now with an already opened IO space.  */
>          var ios = open ("*foo*");