[m-dev.] for review: cleanup of type_ctor_infos, part 4

[Tyson Dowd]

> @@ -189,33 +193,291 @@
>                      new_data = data;
>                      found_forwarding_pointer(data);
>                  }
> +
> +                break;
> +                }
> +            }
> +        } else {
> +            /* XXX CHECKME */

Another one of these.  Who needs to check them?  There is a deep copy
test case in the tests that might help. 

> +
> +                /*
> +                ** The code we want to execute for the MR_SECTAG_REMOTE
> +                ** and MR_SECTAG_NONE cases is the following. However,
> +                ** this code checks the secondary tag location several times
> +                ** and also checks whether exist_info is NULL several times.
> +                ** Since speed is important, we duplicate the code downstream
> +                ** of the first test of each kind. To avoid double maintenance
> +                ** problems, the stuff that is duplicated is macro invocations;
> +                ** each macro is of course defined only once. Note that the
> +                ** initial and final macros have unbalanced parentheses.
> +                */

This is going to make life interesting when I try to merge in my
accurate GC changes.  I don't really like the idea of macros with
unbalanced parentheses (not even when they match up in pairs).  It makes
the macro language a new programming language to learn, instead of just
shorthand for one or more C statements.

> +
> +/*
> +**          case MR_SECTAG_REMOTE:
> +**          case MR_SECTAG_NONE:
> +**
> +**              data_value = (Word *) MR_body(data, ptag);
> +**              if (in_range(data_value)) {
> +**                  const MR_DuFunctorDesc  *functor_desc;
> +**                  const MR_DuExistInfo    *exist_info;
> +**                  int                     sectag;
> +**                  int                     cell_size;
> +**                  int                     cur_slot;
> +**                  int                     arity;
> +**                  int                     num_ti_plain;
> +**                  int                     num_tci;
> +**                  int                     i;
> +**
> +**                  if (ptag_layout->MR_sectag_locn == MR_SECTAG_NONE) {
> +**                      sectag = 0;
> +**                  } else {
> +**                      sectag = data_value[0];
> +**                  }
> +**
> +**                  functor_desc = ptag_layout->MR_sectag_alternatives[sectag];
> +**                  arity = functor_desc->MR_du_functor_orig_arity;
> +**                  exist_info = functor_desc->MR_du_functor_exist_info;
> +**
> +**                  if (ptag_layout->MR_sectag_locn == MR_SECTAG_NONE) {
> +**                      cell_size = arity;
> +**                  } else {
> +**                      cell_size = 1 + arity;
> +**                  }
> +**
> +**                  if (exist_info == NULL) {
> +**                      num_ti_plain = 0;
> +**                      num_tci = 0;
> +**                  } else {
> +**                      num_ti_plain = exist_info->MR_exist_typeinfos_plain;
> +**                      num_tci = exist_info->MR_exist_tcis;
> +**                      cell_size += num_ti_plain + num_tci;
> +**                  }
> +**
> +**                  incr_saved_hp(new_data, cell_size);
> +**
> +**                  if (ptag_layout->MR_sectag_locn == MR_SECTAG_NONE) {
> +**                      cur_slot = 0;
> +**                  } else {
> +**                      MR_field(0, new_data, 0) = sectag;
> +**                      cur_slot = 1;
> +**                  }
> +**
> +**                  for (i = 0; i < num_ti_plain; i++) {
> +**                      MR_field(0, new_data, cur_slot) = (Word)
> +**                          copy_type_info(&data_value[cur_slot],
> +**                              lower_limit, upper_limit);
> +**                      cur_slot++;
> +**                  }
> +**
> +**                  for (i = 0; i < num_tci; i++) {
> +**                      MR_field(0, new_data, cur_slot) = (Word)
> +**                          copy_typeclass_info(&data_value[cur_slot],
> +**                              lower_limit, upper_limit);
> +**                      cur_slot++;
> +**                  }
> +**
> +**                  for (i = 0; i < arity; i++) {
> +**                      MR_field(0, new_data, cur_slot) =
> +**                          copy_arg(data_value, &data_value[cur_slot],
> +**                              type_ctor_info->type_ctor_version,
> +**                              functor_desc, type_info, (const Word *)
> +**                              functor_desc->MR_du_functor_arg_types[i],
> +**                              lower_limit, upper_limit);
> +**                      cur_slot++;
> +**                  }
> +**
> +**                  new_data = (Word) MR_mkword(ptag, new_data);
> +**                  leave_forwarding_pointer(data_ptr, new_data);
> +**              } else {
> +**                  new_data = data;
> +**                  found_forwarding_pointer(data);
> +**              }
> +**              break;
> +*/

What purpose does the comment serve?  Isn't it just going to get out of
sync with the code?

>          case MR_TYPECTOR_REP_NOTAG:
>          case MR_TYPECTOR_REP_NOTAG_USEREQ:
> +        if (type_ctor_info->type_ctor_version <= MR_RTTI_VERSION__USEREQ) {
> +            Word    layout_entry;
> +            Word    *entry_value;
> +            Word    *data_value;
> +            int     data_tag; 
> +
> +            data_tag = MR_tag(data);
> +            data_value = (Word *) MR_body(data, data_tag);
> +
>              layout_entry = type_ctor_info->type_ctor_layout[data_tag];
>              entry_value = (Word *) MR_strip_tag(layout_entry);
> -            new_data = copy_arg(NULL, data_ptr, NULL, type_info, 
> +            new_data = copy_arg(NULL, data_ptr,
> +                    type_ctor_info->type_ctor_version, NULL, type_info,
>                      (Word *) *MR_TYPE_CTOR_LAYOUT_NO_TAG_VECTOR_ARGS(
>                       entry_value), lower_limit, upper_limit);
> +        } else {
> +            /* XXX CHECKME */

Another one.  There are others too in this file and in mercury_tabling.c.

>          case MR_TYPECTOR_REP_STRING:
> +        {
> +            Word    *data_value;
> +            int     data_tag; 
> +
> +            /* XXX simplify: tag should be zero */
> +            data_tag = MR_tag(data);
> +            data_value = (Word *) MR_body(data, data_tag);
> +

As you point out in the comment, this code doesn't need to be here.
Same in the other cases.


> Index: runtime/mercury_type_info.c
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_type_info.c,v
> retrieving revision 1.32
> diff -u -b -r1.32 mercury_type_info.c
> --- runtime/mercury_type_info.c	2000/01/19 09:45:22	1.32
> +++ runtime/mercury_type_info.c	2000/02/24 13:30:39
> @@ -18,7 +18,7 @@
>  static Word *
>  MR_get_arg_type_info(const Word *term_type_info, 
>  	const Word *arg_pseudo_type_info, const Word *data_value, 
> -	const Word *functor_descriptor);
> +	int rtti_version, const MR_DuFunctorDesc *functor_desc);
>  
>  /*---------------------------------------------------------------------------*/
>  
> @@ -66,7 +66,8 @@
>  MR_create_type_info(const Word *term_type_info, const Word *arg_pseudo_type_info)
>  {
>  	return MR_create_type_info_maybe_existq(term_type_info, 
> -		arg_pseudo_type_info, NULL, NULL);
> +		arg_pseudo_type_info, NULL,
> +		MR_RTTI_VERSION__CLEAN_LAYOUT, NULL);
>  }
>  
>  	/*

I'm not sure this is a good idea.  I can see this reference to 
MR_RTTI_VERSION__CLEAN_LAYOUT getting lost in future.  If you are going
to use it here, at least give a comment explaining why it is needed and
when it can be removed.

Same goes for the introduction of the rtti_version as an argument to
various functions. 

-- 
The quantum sort: 
	while (!sorted) { do_nothing(); }
Tyson Dowd   <tyson at tyse.net>   http://tyse.net/
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------