Re: [COMMITTED] pkl: add assertion to `pvm_make_array_type'

[Jose E. Marchesi]

>> Hi Jose.
>>
>> On Fri, Oct 28, 2022 at 01:14:46AM +0200, Jose E. Marchesi wrote:
>>> 
>>> Hi Mohammad.
>>> 
>>> > diff --git a/libpoke/pvm-val.c b/libpoke/pvm-val.c
>>> > index 641fbd8b..54341761 100644
>>> > --- a/libpoke/pvm-val.c
>>> > +++ b/libpoke/pvm-val.c
>>> > @@ -527,12 +527,14 @@ pvm_make_offset_type (pvm_val base_type, pvm_val 
>>> > unit)
>>> >  }
>>> >  
>>> >  pvm_val
>>> > -pvm_make_array_type (pvm_val type, pvm_val bound)
>>> > +pvm_make_array_type (pvm_val type, pvm_val bounder)
>>> >  {
>>> >    pvm_val atype = pvm_make_type (PVM_TYPE_ARRAY);
>>> >  
>>> > +  assert (bounder);
>>> 
>>> I think this ought to be:
>>> 
>>>   assert (bounder != PVM_NULL);
>>> 
>>
>> Oops!
>> Sorry!
>>
>> And fixing it leads to a bunch of failure in `poke.pickles' and
>> `poke.std' directory!
>
> Consider this:
>
> -------------
> diff --git a/libpoke/pvm-val.c b/libpoke/pvm-val.c
> index 54341761..0ce493a3 100644
> --- a/libpoke/pvm-val.c
> +++ b/libpoke/pvm-val.c
> @@ -531,7 +531,7 @@ pvm_make_array_type (pvm_val type, pvm_val bounder)
>  {
>    pvm_val atype = pvm_make_type (PVM_TYPE_ARRAY);
>  
> -  assert (bounder);
> +  assert (PVM_IS_CLS (bounder));
>  
>    PVM_VAL_TYP_A_ETYPE (atype) = type;
>    PVM_VAL_TYP_A_BOUND (atype) = bounder;
> diff --git a/poke/poke.c b/poke/poke.c
> index 7b258410..6385b26a 100644
> --- a/poke/poke.c
> +++ b/poke/poke.c
> @@ -322,6 +322,7 @@ set_script_args (int argc, char *argv[])
>    int i, nargs;
>    uint64_t index;
>    pk_val argv_array;
> +  pk_val array_type_bounder = pk_decl_val (poke_compiler, "_pkl_mkclsn");
>  
>    /* Look for -L SCRIPT */
>    for (i = 0; i < argc; ++i)
> @@ -333,7 +334,7 @@ set_script_args (int argc, char *argv[])
>    nargs = argc - i;
>    argv_array = pk_make_array (pk_make_uint (nargs, 64),
>                                pk_make_array_type (pk_make_string_type (),
> -                                                  PK_NULL /* bound */));
> +                                                  array_type_bounder));
>  
>    for (index = 0; i < argc; ++i, ++index)
>      pk_array_insert_elem (argv_array, index,
> -------------
>
> This works, but:
>
> 1) It makes use of the run-time _pkl_mkclsn, which is supposed to be an
>    internal detail of the compiler run-time.  Also, it is not possible
>    for libpoke users to denote PVM_NULL, so the poke app cannot define a
>    bounder function that returns null, by itself.
>
> 2) pk_make_array_type in libpoke.h is documented to "at this point"
>    always get PK_NULL as the `bound' argument.
>
> We could change the documentation of pk_make_array_type to say something
> like this:
>
>   /* Build and return an array type.
>
>      ETYPE is the type of the elements of the array.
>      BOUNDER is either:
>      - PVM_NULL to denote an unbounded array.
>      - An uint<64> to denote an array type bounded by number of
>        elements.
>      - An offset<uint<64>,1> to denote an array type bounded by size.  */
>
> Then, in libpoke.c:pk_make_array_type we can pk_decl_val and pk_call
> _pkl_mkclsn/_pkl_mkclsi/_pkl_mkclso to actually create the real bounder.
>
> This would keep things simpler for libpoke users.
> For example, poke.c would remain the same as today.
>
> WDYT?

Actually, scratch that idea.  It is a very bad idea.

Hiding this useful abstraction doesn't make any sense.  Poke arrays are
dynamically bounded and that is what makes them awesome :)

So I went with the fix below.
Now you should be able to install the assert in pvm_make_array_type
(checking that bounder is a CLS).

Thanks!

    poke: install proper bounder for argv
    
    2022-10-28  Jose E. Marchesi  <jemarch@gnu.org>
    
            * poke/poke.pk (_pk_bounder_unbounded): New function.
            * poke/poke.c (set_script_args): Install a proper bounder in array
            type.
            * libpoke/libpoke.h: Fix doc string of pk_make_array_type.

diff --git a/ChangeLog b/ChangeLog
index 09bc4ceb..f69c65cf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2022-10-28  Jose E. Marchesi  <jemarch@gnu.org>
+
+       * poke/poke.pk (_pk_bounder_unbounded): New function.
+       * poke/poke.c (set_script_args): Install a proper bounder in array
+       type.
+       * libpoke/libpoke.h: Fix doc string of pk_make_array_type.
+
 2022-10-28  Jose E. Marchesi  <jemarch@gnu.org>
 
        * testsuite/poke.std/std-test.pk: Fix with_temp_ios test for
diff --git a/libpoke/libpoke.h b/libpoke/libpoke.h
index 11271ca5..41dca236 100644
--- a/libpoke/libpoke.h
+++ b/libpoke/libpoke.h
@@ -1068,8 +1068,12 @@ void pk_struct_type_set_ftype (pk_val type, uint64_t idx,
 /* Build and return an array type.
 
    ETYPE is the type of the elements of the array.
-
-   At this point BOUND is always PK_NULL.  */
+   BOUNDER is either:
+   - A ()any closure returning PK_NULL to denote an unbounded array.
+   - A closure ()uint<64> to denote an array type bounded by number of
+     elements.
+   - A closure ()offset<uint<64>,1> to denote an array type bounded by
+     size.  */
 
 pk_val pk_make_array_type (pk_val etype, pk_val bound) LIBPOKE_API;
 
diff --git a/poke/poke.c b/poke/poke.c
index 7b258410..a13d2260 100644
--- a/poke/poke.c
+++ b/poke/poke.c
@@ -322,6 +322,7 @@ set_script_args (int argc, char *argv[])
   int i, nargs;
   uint64_t index;
   pk_val argv_array;
+  pk_val array_type_bounder = pk_decl_val (poke_compiler, 
"pk_bounder_unbounded");
 
   /* Look for -L SCRIPT */
   for (i = 0; i < argc; ++i)
@@ -333,7 +334,7 @@ set_script_args (int argc, char *argv[])
   nargs = argc - i;
   argv_array = pk_make_array (pk_make_uint (nargs, 64),
                               pk_make_array_type (pk_make_string_type (),
-                                                  PK_NULL /* bound */));
+                                                  array_type_bounder));
 
   for (index = 0; i < argc; ++i, ++index)
     pk_array_insert_elem (argv_array, index,
diff --git a/poke/poke.pk b/poke/poke.pk
index 76f16b4e..bebec644 100644
--- a/poke/poke.pk
+++ b/poke/poke.pk
@@ -72,6 +72,14 @@ fun pk_exception_handler = (Exception exception) void:
      }
   }
 
+/* Bounder for constructing unbounded Poke array types from C.  */
+
+fun pk_bounder_unbounded = any:
+{
+  /* XXX use `push null' whenever supported.  */
+  return asm any: ("push 7");
+}
+
 /* Load some poke subsystems.  */
 
 load "pk-table.pk";