[m-rev.] for review: centralize compiler's knowledge of __Unify__ etc names

[Fergus Henderson]

On 24-May-2003, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> compiler/hlds_pred.m:
> 	Add a field to pred_infos that says whether the predicate is a unify,
> 	compare or index predicate, and if so, for which type constructor.
> 	Code that used to test the predicate's name for __Unify__ etc now
> 	tests this field instead. Similarly the code that used to employ
> 	devious tricks to find out the type the unify/compare/index predicate
> 	is for.

Why use a data field rather than a function (which may be accessed
via similar syntax to a data field)?

Using a data field here will increase the compiler's memory usage,
increase GC costs, and reduce locality, quite possibly to a significant
degree.

> compiler/rtti.m:
> 	Include this field in rtti_proc_labels as well as pred_infos.

Likewise (although here it is probably less important).

> compiler/hlds_out.m:
> 	Don't refer to the 'type' that a unify, compare or index predicate
> 	is for; refer to the type *constructor*.

> Index: compiler/hlds_out.m
...
> @@ -371,19 +371,14 @@
>  	{ pred_info_name(PredInfo, Name) },
>  	{ pred_info_arity(PredInfo, Arity) },
>  	{ pred_info_get_is_pred_or_func(PredInfo, PredOrFunc) },
> +	{ pred_info_get_maybe_special_pred(PredInfo, MaybeSpecial) },
>  	( 
> -		{ special_pred_name_arity(Kind, _, Name, Arity) } 
> +		{ MaybeSpecial = yes(SpecialId - TypeCtor) } 
>  	->
> -		{ special_pred_description(Kind, Descr) },
> +		{ special_pred_description(SpecialId, Descr) },
>  		io__write_string(Descr),
> -		io__write_string(" for type "),
> -		{ pred_info_arg_types(PredInfo, TVarSet, _ExistQVars,
> -			ArgTypes) },
> -		( { special_pred_get_type(Name, ArgTypes, Type) } ->
> -			mercury_output_term(Type, TVarSet, no)
> -		;
> -			{ error("special_pred_get_type failed!") }
> -		)
> +		io__write_string(" for type constructor "),
> +		hlds_out__write_type_name(TypeCtor)

This does not seem like an improvement,
since now the message doesn't specify the arity.

In the case where Arity = 0, I think it would be much better to stick
with the simpler wording ("for type" rather than "for type constructor").

Even in the other cases, I'm not sure this is an improvement.
Why say     "unification procedure for type constructor map/2"
rather than "unification procedure for type map(K,V)"?

> +++ compiler/magic_util.m	24 May 2003 10:11:27 -0000
> @@ -654,11 +654,11 @@
>  		% It had better be an in-in unification, since Aditi
>  		% relations cannot have non-ground arguments. This is 
>  		% checked elsewhere.
> -		% XXX __Unify__/2 needs to be special cased in rl_exprn.m 
> -		% because we don't add the type_info arguments.
> +		% XXX unification predicates need to be special cased
> +		% in rl_exprn.m because we don't add the type_info arguments.
>  
>  		hlds_pred__in_in_unification_proc_id(UniProcId),
> -		SymName = unqualified("__Unify__"),
> +		XXX SymName = unqualified("__Unify__"),
>  		ArgVars = [Var, OutputVar],
>  		Test = call(UniPredId, UniProcId, ArgVars, not_builtin,
>  			no, SymName) - GoalInfo

Hmm, isn't that XXX a syntax error?
It looks like the diff that you posted hasn't been compiled.

Otherwise, that looks fine.  But the efficiency issue that I mentioned
at the top regarding the extra data field is an important one, which
should be addressed before this is committed.

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