[m-dev.] for review: RTTI for existential types

[Fergus Henderson]

On 24-Nov-1999, David Glen JEFFERY <dgj at cs.mu.OZ.AU> wrote:
> 
> Implement RTTI for functors with existentially typed arguments.

Great stuff, thanks!

I have a few comments below.  Byt my main criticism is that you
should add some test cases.  Apart from that it looks good.

> compiler/base_type_layout.m:
> 	Generate extra information in the stack layout: the RTTI needs to
> 	include the number of extra arguments added for type infos and
> 	typeclass infos for each functor, as well as the locations of the
> 	type infos for each type.
> compiler/stack_layout.m:
> 	Pass some extra arguments indicating that the pseudo type infos being
> 	handled are not existentially quantified.
> compiler/std_util.m:
> 	Change ML_expand so that it includes information about existentially
> 	quantified arguments in the expand info.
> compiler/mercury_type_info.c:
> 	Use the new information in the RTTI to look up the type info packed
> 	inside a constructor if the pseudo type-info in question is
> 	existentially quantified. This may involve looking inside a typeclass
> 	info or just taking the type info directly.
> compiler/mercury_type_info.h:
> 	Change some prototypes and add macros to access the new information in
> 	the functor descriptor.

The log message here would be a bit easier to read if you used
blank lines between the entry for each file.

> --- base_type_layout.m	1999/07/13 08:52:40	1.46
> +++ base_type_layout.m	1999/11/24 04:54:41
...
> @@ -1183,6 +1379,8 @@
>  :- pred base_type_layout__remove_create(rval, maybe(rval)).
>  :- mode base_type_layout__remove_create(in, out) is det.
>  
> +:- import_module exception.
> +

I think that import is unused.

> +++ mercury_type_info.c	1999/11/24 05:16:15
> @@ -154,23 +154,85 @@
>          ** which does much the same thing, only allocating using MR_GC_malloc()
>          ** instead of on the Mercury heap.
>          */
> -
>  Word * 
>  MR_create_type_info(Word *term_type_info, Word *arg_pseudo_type_info)
>  {
> +        return MR_create_type_info_maybe_existq(term_type_info, 
> +                arg_pseudo_type_info, NULL, NULL);
> +}
> +
> +/* XXX Do I need to change MR_make_type_info too? As far as I can tell,
> + * MR_make_type_info is never called on arguments of constructors, but I don't
> + * know whether that is by design or not... Tyse? Fergus? */

Yes, I think you do.  In particular, MR_make_type_info() gets called
to construct the type_info for a constructor argument in copy_arg()
in runtime/mercury_deep_copy_body.h.

You should add a test case that tests calling deep_copy on existentially
quantified data types.

> +        /*
> +        ** MR_create_type_info_maybe_existq():
> +        **
> +        ** The same as MR_create_type_info except that the type-info being
> +        ** created may be for an existentially typed argument of a constructor.
> +        ** In order to handle this, it also takes the data value from which
> +        ** the values whose pseudo type-info we are looking at was taken, as
> +        ** well as the functor descriptor for that functor
> +        */
> +Word * 
> +MR_create_type_info_maybe_existq(Word *term_type_info, 
> +        Word *arg_pseudo_type_info, Word *data_value, 
> +        Word *functor_descriptor)
> +{
...
> +                if ((Word)arg_pseudo_type_info <= num_univ_type_infos) {
> +                                /* This is a universally quantified type
> +                                 * variable */
> +                        arg_type_info = (Word *) 
> +                                term_type_info[(Word) arg_pseudo_type_info];
> +                }
> +                else {
> +                                /* This is an existentially quantified type
> +                                 * variable */
> +                        Word type_info_locn;

The layout for those comments is not what our coding standards recommend.
It should be

	/*
	** This is a long comment
	** spread over several lines.
	*/
	...


> +                        type_info_locn = 
> +                                ((Word *)MR_TYPE_CTOR_LAYOUT_FUNCTOR_DESCRIPTOR_TYPE_INFO_LOCNS(functor_descriptor))[(Integer)arg_pseudo_type_info - num_univ_type_infos - 1];
>  
> +                        if (type_info_locn && (Unsigned) 1) {
> +
> +                                /* This is indirect; the type-info is inside a
> +                                 * typeclass-info */
> +
> +                                int typeinfo_number;
> +                                int arg_number;
> +
> +                                typeinfo_number = type_info_locn >> 7;
> +
> +                                arg_number = (type_info_locn >> 1) 
> +                                                & (Unsigned) 63;
> +
> +                                arg_type_info = 
> +                                        MR_typeclass_info_type_info(
> +                                                data_value[arg_number],
> +                                                typeinfo_number);

I think you should define macros or use named constants
rather than using magic numbers here.

> +                        /* Maybe we need to thread the value */
> +                        /* down as well... */
>                  equiv_type_info = (Word) MR_create_type_info(
>                                  (Word *) maybe_equiv_type_info, 
>                                  (Word *) equiv_type_info);

Hmm... I'm not sure about that one.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------