[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