[m-rev.] for review: move towards replacing base_typeclass_infos

[Zoltan Somogyi]

On 20-Oct-2003, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> On 20-Oct-2003, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> > Move toward the proposed structures for representing type class information
> > at runtime by adding code for generating the structures corresponding to
> > base_typeclass_infos. The structures corresponding to typeclass_infos will
> > be added in a later change.
> 
> Since this change is incomplete, we should put an entry for it in the
> compiler/notes/work_in_progress.html file.
> 
> Also, it would be a good idea to add a comment to base_typeclass_info.m
> and/or compiler/notes/type_class_transformation.html mentioning the new
> RTTI scheme for type classes.

Done and done.

> > +build_rtti_type_tc_name(type_class_instance_methods(_, _), _Size,
> > +		_GCC_Type) -->
> > +	{ error("build_rtti_type_tc_name: type_class_instance_methods NYI") }.
> 
> s/ error("... NYI") / sorry(this_file, "...") /g

Done.

> > Index: compiler/rtti.m
> 
> An XXX comment here saying which types are not yet (really) used
> would probably be a good idea.

Done.

> > @@ -664,6 +761,10 @@
> >  :- pred tc_rtti_name_java_type(tc_rtti_name::in, string::out, bool::out)
> >  	is det.
> >  
> > +	% Given a type in a type vector in a type class instance declaration,
> > +	% return its string encoding for use in RTTI data structures.
> > +:- func rtti__encode_tc_instance_type(tc_type) = string.
> 
> How would such a string be decoded?

At the moment, it is never decoded. I am not sure that the encoding is fully
invertible, which is why I added an XXX to this effect in the relevant
predicate. I am also not sure if invertibility will ever be actually required.

> Is the string guaranteed to be a valid C identifier,
> or can it contain whitespace, special characters, etc.?

The former. I extended the predicate comment to say so.

> > +% The encoding we use here depends on the types in instance declarations
> > +% being type constructors applied to vectors of distinct variables. When
> > +% we lift that restriction, we will have to change this scheme.
> 
> It might be a good idea to add a Latex comment in
> doc/reference_manual.texi where it specifies this
> restriction, pointing to rtti.m.

Done.

> > +% The code here is based on the code of base_typeclass_info__type_to_string,
> > +% but its input is of type `maybe_pseudo_type_info', not of type `type'.
> > +
> > +rtti__encode_tc_instance_type(TCType) = Str :-
> ...
> > +	% XXX This naming scheme is the same as for base_typeclass_infos.
> > +	% We should think about whether its uniquely invertible.
> > +	string__append_list([TypeStr, "__arity", ArityStr, "__"], Str).
> 
> s/its/it's/
> or (preferably) s/its/it is/

Done.

> > Index: compiler/rtti_out.m
> > +:- pred output_pred_or_func(pred_or_func::in,
> > +	io__state::di, io__state::uo) is det.
> > +
> > +output_pred_or_func(PredOrFunc, !IO) :-
> > +	(
> > +		PredOrFunc = predicate,
> > +		io__write_string("MR_PREDICATE", !IO)
> > +	;
> > +		PredOrFunc = function,
> > +		io__write_string("MR_FUNCTION", !IO)
> > +	).
> 
> That piece of code seems to be duplicated in layout_out.m (lines 906-912).

I have factored out the duplicated code.

> > @@ -1073,6 +1370,15 @@
> >  		io__write_string("#endif /* MR_STATIC_CODE_ADDRESSES */\n",
> >  			!IO)
> >  	;
> > +		Data = type_class_instance(_)
> > +	->
> > +		io__write_string("#ifndef MR_STATIC_CODE_ADDRESSES\n", !IO),
> > +		io__write_string("#error ""type_class_instance " ++
> > +			"not yet supported without static code addresses""\n",
> > +			!IO),
> > +		io__write_string("#endif /* MR_STATIC_CODE_ADDRESSES */\n",
> > +			!IO)
> > +	;
> 
> It looks to me like an XXX comment is warranted there.

Why? The #error isn't a good enough warning?

> > +%		Data = base_typeclass_info(_InstanceModuleName, _ClassId,
> > +%			_InstanceString, _BaseTypeClassInfo)
> > +%	->
> > +%		% XXX Registering base_typeclass_infos by themselves is not
> > +%		% enough. A base_typeclass_info doesn't say which types it
> > +%		% declares to be members of which typeclass, and for now
> > +%		% we don't even have any data structures in the runtime system
> > +%		% to describe such membership information.
> > +%		%
> > +%		% io__write_string(
> > +%		%	"\tMR_register_base_typeclass_info(\n\t\t&"),
> > +%		% output_base_typeclass_info_storage_type_name(
> > +%		%	InstanceModuleName, ClassId, InstanceString, no),
> > +%		% io__write_string(");\n")
> > +%		true
> > +%	;
> 
> Perhaps that commented out code should just be deleted now?

Fine.

> > Index: compiler/type_class_info.m
> 
> I suggest adding an XXX comment here explaining that the RTTI data
> generated by this module is not yet used by the runtime system,
> or something along those lines.

Done.

> > +++ runtime/mercury_type_tables.c	18 Oct 2003 10:19:50 -0000
> > +/*
> > +** This module maintains four data structures: two hash tables and two lists.
> > +** One hash table and one list contain information about type constructors,
> > +** with the elements in the hash table and the list being MR_TypeCtorInfos,
> > +** while the other hash table and list contain information about type
> > +** classes and their instances, with the elements in the hash table and the
> > +** list being MR_TypeClassDeclInfo pointers.
> > +**
> > +** All four data structures are mononotic: you can insert information into
> > +** them, but you cannot remove anything from them.
> 
> s/mononotic/monotonic/
> 
> The documentation should either explain why they are monotonic,
> or that sentence should be preceded by "XXX in the current implementation, ".

Done.

Zoltan.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------