Re: [PATCH] Struct type interface and a bugfix

[Jose E. Marchesi]

Hi Kostas.
    
    Author: kostasch <sdi1600195@di.uoa.gr>
    Date:   Thu Jun 4 13:27:46 2020 +0300
    
    diff --git a/ChangeLog b/ChangeLog
    index a36ca78f..e105e715 100644
    --- a/ChangeLog
    +++ b/ChangeLog
    @@ -1,3 +1,17 @@
    +2020-06-04  Kostas Chasialis <sdi1600195@di.uoa.gr>
    +
    +    * libpoke/libpoke.h (pk_make_struct_type): Prototype.

Tab before *.

    +   (pk_struct_type_name) : Likewise.
    +   (pk_struct_type_nfields) : Likewise.
    +   (pk_struct_type_fname) : Likewise.
    +   (pk_struct_type_ftype) : Likewise.
    +       (pk_struct_nmethods) : Removed.
    +       (pk_struct_set_method_value) : Likewise.
    +       (pk_struct_method_name) : Likewise.
    +       (pk_struct_method_value) : Likewise.
    +       (pk_struct_method_set_value) : Likewise.
    +   * libpoke/pk-val.c (pk_make_struct_type) : Define.
    +   (pk_struct_type_name) : Define.
    +   (pk_struct_type_nfields) : Define.
    +   (pk_struct_type_fname) : Define.
    +   (pk_struct_type_ftype) : Define.
    +       (pk_struct_nmethods) : Removed.
    +       (pk_struct_set_method_value) : Likewise.
    +       (pk_struct_method_name) : Likewise.
    +       (pk_struct_method_value) : Likewise.
    +       (pk_struct_method_set_value) : Likewise.
    +   * libpoke/pvm-val.c (pvm_allocate_struct_attrs) : Bugfix.

    diff --git a/libpoke/libpoke.h b/libpoke/libpoke.h
    index d149dd39..1ba6a6fc 100644
    --- a/libpoke/libpoke.h
    +++ b/libpoke/libpoke.h
    @@ -490,29 +490,20 @@ pk_val pk_offset_unit (pk_val val);
     /* Structs. */
    
     /* Build and return a poke struct.
    -   NFIELDS is an uint<64> PK value specifying the number of fields
    -   in the struct.  This can be uint<64>0 for an empty struct.
    -
    -   NMETHODS is an uint<64> PK vlaue specifying the number of methods
    -   in the struct.
    +   NFIELDS is an uint<64> PK value specifying the number of fields
    +   in the struct.  This can be uint<64>0 for an empty struct.
    
        TYPE is a type PK value specifying the type of the array.

... of the struct.
    
        The fields and methods in the created struct are initialized to
        PK_NULL.*/

No need to mention the methods any longer.

    -pk_val pk_make_struct (pk_val nfields, pk_val nmethods, pk_val type);
    +pk_val pk_make_struct (pk_val nfields, pk_val type);
    
     /* Get the number of fields of a struct. */
    
     pk_val pk_struct_nfields (pk_val sct);
    
    -/* Get the number of methods in a struct.
    -
    -   SCT is the struct value. */
    -
    -pk_val pk_struct_nmethods (pk_val sct);
    -
     /* Get the bit-offset of the field of a struct, relative to the
        beginning of the struct.
    
    @@ -562,35 +553,7 @@ pk_val pk_struct_field_value (pk_val sct, uint64_t 
idx);
    
     void pk_struct_set_field_value (pk_val sct, uint64_t idx, pk_val value);
    
    -/* Get the NAME of the struct method.
    -
    -   SCT is the struct value.
    -   IDX is the index of the method in the struct. */
    -
    -pk_val pk_struct_method_name (pk_val sct, uint64_t idx);
    -
    -/* Set the NAME of the struct method.
    -
    -   SCT is the struct value.
    -   IDX is the index of the method in the struct.
    -   NAME is the string name for this method. */
    -
    -void pk_struct_set_method_name (pk_val sct, uint64_t idx, pk_val name);
    -
    -/* Get the VALUE of the struct method.
    -
    -   SCT is the struct value.
    -   IDX is the index of the method in the struct. */
    -
    -pk_val pk_struct_method_value (pk_val sct, uint64_t idx);
    -
    -/* Set the VALUE of the struct method.
    -
    -   SCT is the struct value.
    -   IDX is the index of the method in the struct.
    -   VALUE is a PK closure. */
    
    -void pk_struct_set_method_value (pk_val sct, uint64_t idx, pk_val value);
    
    
    +/* Struct types. */
    +
    +/* Build and return a struct type.
    +
    +   NFIELDS is the number of struct fields on this struct.
    +
    +    NAME is a string containing the name of the struct type.

Extra blank.

    +   FNAMES is a C array containing the name of each struct field.
    +
    +   FTYPES is a C array containing the types of each struct field. */
    +
    +pk_val pk_make_struct_type (pk_val nfields, pk_val name, pk_val *fnames, 
pk_val *ftypes);
    +
    +/* Get the name of the struct type.  */
    +
    +pk_val pk_struct_type_name (pk_val type);
    +
    +/* Get the number of fields of the struct type.  */
    +
    +pk_val pk_struct_type_nfields (pk_val type);
    +
    +/* Get the name of a field in the struct type.
    +
    +   TYPE is the struct type.
    +   IDX is the index of the struct field. */
    +
    +pk_val pk_struct_type_fname (pk_val type, uint64_t idx);
    +
    +/* Get type of a field in the struct.
    +
    +   TYPE is the struct type.
    +   IDX is the index of the struct field.  */
    +
    +pk_val pk_struct_type_ftype (pk_val type, uint64_t idx);
    
    
    
    diff --git a/libpoke/pk-val.c b/libpoke/pk-val.c
    index 4769f9b5..b1fcc85e 100644
    --- a/libpoke/pk-val.c
    +++ b/libpoke/pk-val.c
    @@ -17,6 +17,7 @@
      */
    
     #include <config.h>
    
     #include "pvm.h"
     #include "pvm-val.h" /* XXX */
    @@ -183,9 +184,9 @@ pk_type_code (pk_val val)
     }
    
     pk_val
    -pk_make_struct (pk_val nfields, pk_val nmethods, pk_val type)
    +pk_make_struct (pk_val nfields, pk_val type)
     {
    -  return pvm_make_struct(nfields, nmethods, type);
    +  return pvm_make_struct(nfields, pvm_make_ulong (0, 64), type);
     }
    
     pk_val
    @@ -194,12 +195,6 @@ pk_struct_nfields (pk_val sct)
       return PVM_VAL_SCT_NFIELDS (sct);
     }
    
    -pk_val
    -pk_struct_nmethods (pk_val sct)
    -{
    -  return PVM_VAL_SCT_NMETHODS (sct);
    -}
    -
     pk_val pk_struct_field_boffset (pk_val sct, uint64_t idx)
     {
       return PVM_VAL_SCT_FIELD_OFFSET (sct, idx);
    @@ -207,16 +202,20 @@ pk_val pk_struct_field_boffset (pk_val sct, uint64_t
    idx)
    
     void pk_struct_set_field_boffset (pk_val sct, uint64_t idx, pk_val offset)
     {
    +    /*NOTE to jemarch: please tell me if you dont agree with this
    pvm_assert*/
    +  pvm_assert(pk_type_code(pk_typeof(offset)) == PK_OFFSET);

I would not include the assert, as this is library code.

       PVM_VAL_SCT_FIELD_OFFSET (sct, idx) = offset;
     }
    
     pk_val pk_struct_field_name (pk_val sct, uint64_t idx)
    -{
    +{

Spurious change?

       return PVM_VAL_SCT_FIELD_NAME (sct, idx);
     }
    
     void pk_struct_set_field_name (pk_val sct, uint64_t idx, pk_val name)
     {
    +  /*NOTE to jemarch: please tell me if you dont agree with this 
pvm_assert*/
    +  pvm_assert(pk_type_code(pk_typeof(name)) == PK_STRING);

See above.

       PVM_VAL_SCT_FIELD_NAME (sct, idx) = name;
     }
    
    @@ -227,29 +226,12 @@ pk_val pk_struct_field_value (pk_val sct, uint64_t 
idx)
    
     void pk_struct_set_field_value (pk_val sct, uint64_t idx, pk_val value)
     {
    +  /*NOTE to jemarch: please tell me if you dont agree with this 
pvm_assert*/
    +  pk_val sct_type = PVM_VAL_SCT_TYPE (sct);
    +  pvm_assert(pk_type_code(pk_typeof(value)) ==
    pk_type_code(PVM_VAL_TYP_S_FTYPE(sct_type, idx)));

See above.

       PVM_VAL_SCT_FIELD_VALUE (sct, idx) = value;
     }
    
    -pk_val pk_struct_method_name (pk_val sct, uint64_t idx)
    -{
    -  return PVM_VAL_SCT_METHOD_NAME (sct, idx);
    -}
    -
    -void pk_struct_set_method_name (pk_val sct, uint64_t idx, pk_val name)
    -{
    -  PVM_VAL_SCT_METHOD_NAME (sct, idx) = name;
    -}
    -
    -pk_val pk_struct_method_value (pk_val sct, uint64_t idx)
    -{
    -  return PVM_VAL_SCT_METHOD_VALUE (sct, idx);
    -}
    -
    -void pk_struct_set_method_value (pk_val sct, uint64_t idx, pk_val value)
    -{
    -  PVM_VAL_SCT_METHOD_VALUE (sct, idx) = value;
    -}
    -
     pk_val
     pk_make_array (pk_val nelem, pk_val array_type)
     {
    @@ -322,6 +304,36 @@ pk_array_type_bound (pk_val type)
       return PVM_VAL_TYP_A_BOUND (type);
     }
    
    +pk_val
    +pk_make_struct_type (pk_val nfields, pk_val name, pk_val *fnames, pk_val
    *ftypes)
    +{
    +  return pvm_make_struct_type (nfields, name, fnames, ftypes);
    +}
    +
    +pk_val
    +pk_struct_type_name (pk_val type)
    +{
    +  return PVM_VAL_TYP_S_NAME (type);
    +}
    +
    +pk_val
    +pk_struct_type_nfields (pk_val type)
    +{
    +  return PVM_VAL_TYP_S_NFIELDS (type);
    +}
    +
    +pk_val
    +pk_struct_type_fname (pk_val type, uint64_t idx)
    +{
    +  return PVM_VAL_TYP_S_FNAME (type, idx);
    +}
    +
    +pk_val
    +pk_struct_type_ftype (pk_val type, uint64_t idx)
    +{
    +  return PVM_VAL_TYP_S_FTYPE (type, idx);
    +}
    
    
    diff --git a/libpoke/pvm-val.c b/libpoke/pvm-val.c
    index a2172f70..4490f1e4 100644
    --- a/libpoke/pvm-val.c
    +++ b/libpoke/pvm-val.c
    @@ -352,7 +352,7 @@ void
     pvm_allocate_struct_attrs (pvm_val nfields,
                                pvm_val **fnames, pvm_val **ftypes)
     {
    -  size_t nbytes = sizeof (pvm_val) * PVM_VAL_ULONG (nfields) * 2;
    +  size_t nbytes = sizeof (pvm_val) * PVM_VAL_ULONG (nfields);

This is OK.

       *fnames = pvm_alloc (nbytes);
       *ftypes = pvm_alloc (nbytes);
     }