[m-dev.] for review: cleanup of type_ctor_infos, part 0

Fergus Henderson fjh at cs.mu.OZ.AU
Fri Feb 25 19:06:35 AEDT 2000


On 25-Feb-2000, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> Index: mercury_type_info.h

> +#define	MR_PTAG_VALUES		(1 << LOW_TAG_BITS)

It would be a good idea to document the intended meaning
of that macro.

Also it might be clearer to rename it MR_NUM_PTAG_VALUES.

> +** The intention is that if you have a word in an equivalence type that you
> +** want to interpret, you expand the pseudo typeinfo into a real typeinfo,
> +** use that to interpret the word.

s/use that/and then use that/

> +/*
> +** This type describes the layout in any kind of discriminated union
> +** type: du, enum and notag. In an equivalence type, it gives the identity
> +** of the equivalent-to type.
> +*/
> +
> +typedef	union {
> +	Integer			layout_init;
> +	MR_DuTypeLayout		layout_du;
> +	MR_EnumTypeLayout	layout_enum;
> +	MR_NotagTypeLayout	layout_notag;
> +	MR_EquivLayout		layout_equiv;
> +} MR_TypeLayout;

What's `layout_init' for?
The documentation does not mention that.

> +typedef	union {
> +	Integer			functors_init;
> +	MR_DuFunctorDesc	**functors_du;
> +	MR_EnumFunctorDesc	**functors_enum;
> +	MR_NotagFunctorDesc	*functors_notag;
> +} MR_TypeFunctors;

Likewise here.

> +#define	MR_TypeCtorInfo_struct	MR_TypeCtorInfo_Struct

Wouldn't it make more sense to put that #define
in runtime/mercury_bootstrap.h?

> % File: base_type_info.m.

It would be nice to have some more comments in the code
in this module.  There's about 500 lines of code at the
end of this module with only about 3 lines of comments.
While the code itself is fairly readable, I think that
adding some comments could still improve things quite
a bit.

...
> % XXX See polymorphism.m for a description of the various ways to represent
> % type information, including a description of the type_ctor_info structures.

What's the "XXX" for?

base_type_layout.m:
> 	% Maximum value of an integer representation of a variable.
> :- pred base_type_layout__pseudo_typeinfo_max_var(int::out) is det.

I suggest s/variable/type variable/

Also perhaps s/max_var/max_tvar/ ?

Also, it might be better to make this a function
rather than a predicate.

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