[m-dev.] cvs diff: type_info code.

[Tyson Richard DOWD]

> Tyson Richard DOWD, you wrote:
> > 
> > Fergus, could you please review this?
> 
> Sure.
> 
> BTW, your code was generally very well-documented --- thanks.
> 
> General comment: I'm not very comfortable with all this unnamed type
> stuff.  If you can get at it using type_of/1, and do all this sort of
> stuff with it, then I think perhaps it would be better to name it.  The
> name to use would be `void'; it should probably be declared and defined
> in mercury_builtin.m.
> 
> I think it would also be a good idea to enable the code in typecheck.m
> that reports errors for uninstantiated type variables, with the error
> changed to a warning (e.g. "Warning: type defaults to `void'.").
> 
> It would probably also be a good idea to avoid the special representation
> as null typeinfo pointers and instead just use an ordinary typeinfo.
> 
> But don't worry about all that before committing.  I might have a go
> at doing that stuff after you've committed it.

Okay.

> > +int 
> > +get_num_functors(Word type_info)
> > +{
> > +	Word *base_type_functors;
> > +	int Functors;
> > +
> > +	base_type_functors = MR_BASE_TYPEINFO_GET_TYPEFUNCTORS(
> > +		MR_TYPEINFO_GET_BASE_TYPEINFO((Word *) type_info));
> > +
> > +	switch ((int) MR_TYPEFUNCTORS_INDICATOR(base_type_functors)) {
> ...
> > +		case MR_TYPEFUNCTORS_UNIV:
> > +			fatal_error(""std_util:get_num_functors :""
> > +				"" found univ"");
> > +		break;
> > +
> > +		default:
> > +			fatal_error(""std_util:get_num_functors :""
> > +				"" unknown indicator"");
> 
> Hmm, shouldn't you just return Functors = -1 for those cases?

For univ, yes, but not for default - that indicates an internal
error. I'd rather catch it than let it return a possibly incorrect
result.

> 
> Also, the indentation of your `break' statements is non-standard here
> and elsewhere.
> 
> > +	 * If the substituted type parameters from the term_type_info
> > +	 * were type variables, they will be replaced with references
> > +	 * to the unnamed type (''/0).
> 
> I don't understand why that is done and I don't think it should be done.
>

If the type variables remain unbound, we can either
	- replace them with references to ''
	- replace them with NULL pointers, and check for null pointers
	  in any code that uses type_infos or univs, then act
	  appropriately

You said above -
	> It would probably also be a good idea to avoid the special 
	> representation as null typeinfo pointers and instead just use 
	> an ordinary typeinfo.

I thought that '' was this ordinary typeinfo.

Perhaps we should discuss this more.

-- 
       Tyson Dowd           #
                            #             Sign on refrigerator:
     trd at cs.mu.oz.au        #           Refrigerate after opening. 
http://www.cs.mu.oz.au/~trd #                  - C. J. Owen.