[m-dev.] for review: MR_TypeInfo cleanup, part 1

[Fergus Henderson]

On 22-Mar-2000, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> +++ library/std_util.m	2000/03/20 08:06:02
...
> +/*
> +** Values of type `std_util:type_desc' are represented the same way as
> +** values of type `private_builtin:type_info'. Some parts of the library
> +** (e.g. the gc initialization code) depend on this.

It would be helpful to give a pointer here to the documentation
describing the representation of `private_builtin:type_info'.

> +** Values of type `std_util:type_ctor_desc' are not guaranteed to be
> +** represented the same way as values of type `private_builtin:type_ctor_info'.
> +** The representations *are* in fact identical for first order types, but they
> +** differ for higher order types. Instead of a type_ctor_desc being a structure
> +** containing a pointer to the type_ctor_info for pred/0 or func/0 and an
> +** arity, we have a single small encoded integer. This integer is double
> +** the arity, plus zero or one; plus zero encodes a predicate, plus one encodes
> +** a function.
> +**
> +** The maximum arity that can be encoded should be set to twice the maximum
> +** number of general purpose registers, since an predicate or function having
> +** more arguments that this would run out of registers when passing the input
> +** arguments, or the output arguments, or both.
> +*/
> +
> +typedef struct {
> +    Unsigned    type_ctor_desc_dummy_field;
> +} *MR_TypeCtorDesc;

Hmm... I don't understand this struct definition;
what is the purpose of the "dummy" field?

> +#define MR_MAX_HO_ARITY         (2 * MAX_VIRTUAL_REG)
> +
> +#define MR_TYPE_CTOR_INFO_HO_PRED                                       \
> +        ((MR_TypeCtorInfo) &mercury_data___type_ctor_info_pred_0)
> +#define MR_TYPE_CTOR_INFO_HO_FUNC                                       \
> +        ((MR_TypeCtorInfo) &mercury_data___type_ctor_info_func_0)
> +#define MR_TYPE_CTOR_INFO_IS_HO_PRED(T)                                 \
> +        (T == MR_TYPE_CTOR_INFO_HO_PRED)
> +#define MR_TYPE_CTOR_INFO_IS_HO_FUNC(T)                                 \
> +        (T == MR_TYPE_CTOR_INFO_HO_FUNC)
> +#define MR_TYPE_CTOR_INFO_IS_HO(T)                                      \
> +        (MR_TYPE_CTOR_INFO_IS_HO_FUNC(T) || MR_TYPE_CTOR_INFO_IS_HO_PRED(T))

It is a little bit confusing that these macros come here,
in between the definition of the MR_TypeCtorDesc type and
the macros which operate on it.  I think it would be clearer
to move these macros elsewhere, and perhaps to add a comment saying
that they are operations on private_builtin:type_infos.

> +#define MR_TYPECTOR_DESC_UNSIGNED(T)                                    \
> +        ( (Unsigned) &(T)->type_ctor_desc_dummy_field )
...
> +#define MR_TYPECTOR_DESC_IS_HIGHER_ORDER(T)                             \
> +        ( MR_TYPECTOR_DESC_UNSIGNED(T) <= (2 * MR_MAX_HO_ARITY + 1) )
> +#define MR_TYPECTOR_DESC_MAKE_PRED(Arity)                               \
> +        ( (MR_TypeCtorDesc) ((Arity) * 2) )
> +#define MR_TYPECTOR_DESC_MAKE_FUNC(Arity)                               \
> +        ( (MR_TypeCtorDesc) ((Arity) * 2 + 1) )

Hmm... here you just return the number directly, but 
the MR_TYPECTOR_DESC_IS_HIGHER_ORDER() macro calls
MR_TYPECTOR_DESC_UNSIGNED(), which assumes that its argument
will be a pointer to a struct containing the number as its first field.

So `MR_TYPECTOR_DESC_IS_HIGHER_ORDER(MR_TYPECTOR_DESC_MAKE_PRED(0))'
would lead to a seg fault.  That sure looks to me like a bug.

Shouldn't MR_TYPECTOR_DESC_UNSIGNED(T) be defined as just `((Unsigned)(T))'?
(If that is so, it would probably be clear to just write it inline,
rather than defining it as a macro.)

> +#define MR_TYPECTOR_DESC_GET_FIRST_ORDER_TYPE_CTOR_INFO(T)              \
> +        ( (MR_TypeCtorInfo) &(T)->type_ctor_desc_dummy_field )
...
> +#define MR_TYPECTOR_DESC_MAKE_FIRST_ORDER(type_ctor_info)               \
> +        ( (MR_TypeCtorDesc) &type_ctor_info->arity )

These look a bit wierd to me too.  Why are the indirections there?
Shouldn't these just be

	#define MR_TYPECTOR_DESC_MAKE_FIRST_ORDER(type_ctor_info)          \
		((MR_TypeCtorDesc) type_ctor_info)
	#define MR_TYPECTOR_DESC_GET_FIRST_ORDER(type_ctor_desc)          \
		((MR_TypeCtorInfo) type_ctor_desc)

?

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