[m-rev.] for review: base_typeclass_info rtti refs

[Fergus Henderson]

This change looks like a good cleanup.
I do have some minor comments -- see below.

On 01-May-2003, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> Index: compiler/rtti.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/compiler/rtti.m,v
> retrieving revision 1.29
> diff -u -b -r1.29 rtti.m
> --- compiler/rtti.m	21 Mar 2003 05:52:07 -0000	1.29
> +++ compiler/rtti.m	30 Apr 2003 06:31:34 -0000
> @@ -480,6 +480,10 @@
>  			base_typeclass_info
>  		).
>  
> +:- type rtti_id
> +	--->	rtti_id(rtti_type_ctor, rtti_name)
> +	;	tc_rtti_id(tc_rtti_name).
> +
>  :- type rtti_name

A comment or two here might help.

Also, I think it would be clear to name the constructor for the
first alternative as "name_rtti_id".

> @@ -521,8 +527,7 @@
>  	% Convert a rtti_data to a rtti_type_ctor and a rtti_name.
>  	% This calls error/1 if the argument is a type_var/1 rtti_data,
>  	% since there is no rtti_type_ctor to return in that case.
> -:- pred rtti_data_to_name(rtti_data::in, rtti_type_ctor::out, rtti_name::out)
> -	is det.
> +:- pred rtti_data_to_id(rtti_data::in, rtti_id::out) is det.

The comment there about calling error/1 is no longer valid.

>  	% Convert an id that specifies a kind of variable arity type_info
>  	% or pseudo_type_info into the type_ctor of the canonical (arity-zero)
> @@ -530,12 +535,26 @@
>  :- func var_arity_id_to_rtti_type_ctor(var_arity_ctor_id) = rtti_type_ctor.
>  
>  	% return yes iff the specified rtti_name is an array
> +:- func rtti_id_has_array_type(rtti_id) = bool.

s/rtti_name/rtti_id/

> +	% return yes iff the specified rtti_name is an array
> +:- func tc_rtti_name_has_array_type(tc_rtti_name) = bool.

In the comment, s/rtti_name/tc_rtti_name/

> +	% Return yes iff the specified rtti_name should be exported
> +	% for use by other modules.
> +:- func rtti_id_is_exported(rtti_id) = bool.

In the comment, s/rtti_name/rtti_id/

> @@ -610,8 +628,16 @@
>  
>          % Return true iff the given type of RTTI data structure includes
>  	% code addresses.
> +:- func rtti_id_would_include_code_addr(rtti_id) = bool.
> +
> +        % Return true iff the given type of RTTI data structure includes
> +	% code addresses.
>  :- func rtti_name_would_include_code_addr(rtti_name) = bool.
>  
> +        % Return true iff the given type of RTTI data structure includes
> +	% code addresses.
> +:- func tc_rtti_name_would_include_code_addr(tc_rtti_name) = bool.
>
>          % Return true iff the given type_info's RTTI data structure includes
>  	% code addresses.
>  :- func type_info_would_incl_code_addr(rtti_type_info) = bool.

It might be clearer to include the comment just once, and group the
declarations:

		% Return true iff the given type of RTTI data structure
		% includes code addresses.
	:- func rtti_id_would_include_code_addr(rtti_id) = bool.
	:- func rtti_name_would_include_code_addr(rtti_name) = bool.
	:- func tc_rtti_name_would_include_code_addr(tc_rtti_name) = bool.
	:- func type_info_would_incl_code_addr(rtti_type_info) = bool.

Actually the same goes for the other functions like this.

> Index: compiler/rtti_out.m
> @@ -556,7 +553,7 @@
>  	io__write_string("(MR_PseudoTypeInfo *) "), % cast away const
>  	(
>  		{ ArgInfos = [_ | _] },
> -		output_addr_of_rtti_addr(RttiTypeCtor, field_types(Ordinal))
> +		output_addr_of_rtti_addr( RttiTypeCtor, field_types(Ordinal))

s/( /(/

> +output_rtti_addr_storage_type_name(RttiId, BeingDefined) -->

That should probably be renamed output_rtti_id_storage_type_name.
I don't think it has anything to do with rtti_addrs anymore.

> +:- pred output_addr_of_rtti_addr(rtti_type_ctor::in, rtti_name::in,
> +	io__state::di, io__state::uo) is det.
> +
> +output_addr_of_rtti_addr(RttiTypeCtor, RttiName) -->
> +	output_addr_of_rtti_id(rtti_id(RttiTypeCtor, RttiName)).

Likewise here the use of rtti_addr is historical and no longer meaningful.
I suggest renaming this output_addr_of_name_rtti_id.

> +:- pred output_rtti_addr(rtti_type_ctor::in, rtti_name::in,
> +	io__state::di, io__state::uo) is det.
> +
> +output_rtti_addr(RttiTypeCtor, RttiName) -->
> +	output_rtti_id(rtti_id(RttiTypeCtor, RttiName)).

Likewise here, I suggest renaming to output_name_rtti_id.

[... to be continued ...]

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------