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

[Zoltan Somogyi]

On 24-Mar-2000, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> > +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?

The purpose is to make sure that calling MR_TYPECTOR_DESC_UNSIGNED, which
is defined as

#define MR_TYPECTOR_DESC_UNSIGNED(T)
        ( (Unsigned) &(T)->type_ctor_desc_dummy_field )

gets an error from the C compiler if invoked with something other than a
MR_TypeCtorDesc, and similarly for the other base-level macro that knows
about the field, MR_TYPECTOR_DESC_GET_FIRST_ORDER_TYPE_CTOR_INFO.

> > +#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.

No, they are not; they are operations on private_builtin:type_*ctor*_infos.
I have moved them back to mercury_type_info.h.

> > +#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.

No, this is not a bug. MR_TYPECTOR_DESC_UNSIGNED() *does not* assume that
its argument will be a pointer to a struct containing the number as its
first field; it assumes that its argument is a value of type MR_TypeCtorDesc
(i.e. a pointer) which it then simply casts to Unsigned. The
MR_TYPECTOR_DESC_UNSIGNED macro does not do any dereferencing;
it returns the *address* of the dummy field, not its contents.

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

That would lose the error-detection capability.

> > +#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)

Same reason; I want to make sure that you will get an error if you give these
macros an argument of the wrong type. One of the major things wrong with the
previous RTTI scheme was that it did not give you any of this type of help
in making sure that your code did not do wrong things. With the current scheme,
the first compiled version bootstrapped successfully, and only had problems
with three test cases (which I have since fixed, of course).

I will document this scheme better before I commit.

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