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

Fergus Henderson fjh at cs.mu.OZ.AU
Wed Dec 8 20:28:07 AEDT 1999


On 07-Dec-1999, David Glen JEFFERY <dgj at cs.mu.OZ.AU> wrote:
> Here is a diff which addresses your previous comments, including implementing
> deep copying for existentially typed functors.

That looks good. 

The log message did not mention the test case.

I also have a lot of minor comments about the layout.

> --- base_type_layout.m	1999/07/13 08:52:40	1.46
> +++ base_type_layout.m	1999/12/01 07:20:58
> @@ -717,9 +728,15 @@
>          base_type_layout__no_tag_indicator(yes, NoTagIndicator),
>          Rval0 = yes(const(int_const(NoTagIndicator))),
>  
> +        base_type_layout__get_type_id(LayoutInfo0, _-NumUnivQTvars),

Please put whitespace around operators.
i.e.  s/-/ - /

> +base_type_layout__generate_type_info_locns(Tvars, Constraints, Locns, 
> +                NUnconstrained, NumConstraints, Info) :-
...
> +        list__foldl((pred(T::in, N0-Ls0::in, N-Ls::out) is det :- 

Likewise here.

> +                        make_direct_typeinfo_index(N0, Locn),
> +                        map__det_insert(Ls0, T, Locn, Ls),
> +                        N = N0 + 1
> +                ), UnconstrainedTvars, 0-Locns0, NUnconstrained-Locns1),

And here.

> +first_matching_type_class_info([C|Cs], Tvar, MatchingConstraint, N0, N,
> +                TypeInfoIndex) :-
> +        C = constraint(_, Ts), 
> +        term__vars_list(Ts, TVs),
> +        (
> +                list__nth_member_search(TVs, Tvar, Index)
> +        ->
> +                N = N0,
> +                MatchingConstraint = C,
> +                TypeInfoIndex = Index
> +        ;
> +                first_matching_type_class_info(Cs, Tvar, MatchingConstraint,
> +                        N0+1, N, TypeInfoIndex)

And here.

> +++ std_util.m	1999/12/02 08:27:36
> @@ -274,7 +274,7 @@
>          %
>          % Warning: support for existential types is still experimental.
>          %
> -:- some([T], pred has_type(T::unused, type_info::in) is det).
> +:- some [T] pred has_type(T::unused, type_info::in) is det.

The comment above that code should probably be deleted now.

> @@ -2243,9 +2248,12 @@
>  
>                          arg_pseudo_type_info = (Word *)
>                              MR_TYPE_CTOR_LAYOUT_FUNCTOR_DESCRIPTOR_ARGS(
> -                                    functor_descriptor)[i];
> -                        info->type_info_vector[i] = (Word) MR_create_type_info(
> -                            type_info, arg_pseudo_type_info);
> +                                        functor_descriptor)[i];
> +                        info->type_info_vector[i] = 
> +                            (Word) MR_create_type_info_maybe_existq(
> +                                        type_info, arg_pseudo_type_info, 
> +                                        (Word *)data_value,
> +                                        functor_descriptor);

s/(Word *)data_value/(Word *) data_value/
                             ^

> @@ -2287,8 +2296,11 @@
>                      arg_pseudo_type_info = (Word *)
>                          MR_TYPE_CTOR_LAYOUT_FUNCTOR_DESCRIPTOR_ARGS(
>                                  functor_descriptor)[i];
> -                    info->type_info_vector[i] = (Word) MR_create_type_info(
> -                        type_info, arg_pseudo_type_info);
> +                    info->type_info_vector[i] = 
> +                        (Word) MR_create_type_info_maybe_existq(
> +                            type_info, arg_pseudo_type_info, 
> +                                        (Word *)data_value,
> +                                        functor_descriptor);

Likewise.

> +++ mercury_deep_copy_body.h	1999/12/06 06:57:35
> +                        MR_field(0, new_data, i + num_extra_args+ 1) 

s/+/ +/

> +static Word
> +copy_typeclass_info(maybeconst Word *typeclass_info_ptr, 
> +        const Word *lower_limit, const Word *upper_limit)
> +{
> +        Word *typeclass_info = (Word *) *typeclass_info_ptr;
> +
> +        if (in_range(typeclass_info)) {
> +                Word *base_typeclass_info;
> +                Word *new_typeclass_info;
> +                Integer arity, num_super, num_arg_typeinfos, offset, i;
> +
> +                /*
> +                ** Note that we assume base_typeclass_infos will always be
> +                ** allocated statically, so we never copy them.
> +                */
> +
> +                base_typeclass_info = (Word *)*typeclass_info;

Use a space after the cast.

> +                num_arg_typeinfos = MR_typeclass_info_num_type_infos(typeclass_info);

That line is > 80 columns.

> +                        (Word) new_typeclass_info);
> +                return (Word)new_typeclass_info;
> +        } else {
> +                found_forwarding_pointer(typeclass_info);
> +                return (Word)typeclass_info;

Use a space after the casts.

> +Word *
> +MR_get_arg_type_info(Word *term_type_info, 
> +        Word *arg_pseudo_type_info, Word *data_value, 
> +        Word *functor_descriptor)
> +{
> +        Word *arg_type_info;
> +
> +        int num_univ_type_infos;
> +
> +        num_univ_type_infos =
> +                MR_TYPEINFO_GET_TYPE_CTOR_INFO(term_type_info)->arity;
> +
> +        if ((Word)arg_pseudo_type_info <= num_univ_type_infos) {

Likewise here.

> +        }
> +        else {

The `else' should be on the same line as the `}'.

> +                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];

That line is > 80 columns.

> +                if (MR_TYPE_INFO_LOCN_IS_INDIRECT(type_info_locn)) {
> +
> +                        /* This is indirect; the type-info
> +                        ** is inside a typeclass-info 
> +                        */

The comment here should have the `/*' on a line of its own.

> +                }
> +                else {

The `else' should be on the same line as the `}'.

> @@ -370,6 +446,8 @@
>                  /* Look past equivalences */
>          while (MR_TYPE_CTOR_FUNCTORS_INDICATOR(functors) == MR_TYPE_CTOR_FUNCTORS_EQUIV) {
>                  equiv_type_info = (Word) MR_TYPE_CTOR_FUNCTORS_EQUIV_TYPE(functors);
> +                        /* 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);

That comment is not properly formatted.

More importantly, I don't know exactly what the comment means
by "the value"... and the comment certainly doesn't inspire confidence
in the code.

-- 
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
--------------------------------------------------------------------------



More information about the developers mailing list