[m-dev.] For review: making terms polymorphic

[Fergus Henderson]

On 04-Nov-1998, Thomas Charles CONWAY <conway at cs.mu.OZ.AU> wrote:
...
> +++ NEWS	1998/11/03 21:38:07
> @@ -319,6 +319,11 @@
>    See the "benchmarking" chapter of the Mercury Library Reference Manual
>    for details.
>  
> +* Term, var, var_supply and varset are now polymorphic. This allows one to
> +  distinguish between terms, etc denoting different kinds of things by
> +  making them differently typed. The new coercion predicates listed below
> +  allow one to coerce terms, etc between types.

s/Term, var, var_supply and varset/The types `term', `var', .../
s/etc/etc./

I also suggest
	s/making them differently typed/giving them different types/

The log message and the news file changes don't mention
that you've left the existing monomorphic versions around.

Since the change is not completely backwards compatible, the
news file should explain what people will need to do to fix
their code.

> +	% In the absence of type qualifiers, this predicate is useful for
> +	% avoiding unbound type variables.
> +
> +:- pred term__generic_term(term).
> +:- mode term__generic_term(in) is det.

A comment here saying what the predicate does (not just what it is
useful for) would be helpful.

"In the absence of type qualifiers" is not going to be clear
to most people.

> +varset__coerce(varset(S0, N0, B0), varset(S, N, B)) :-
> +	term__coerce_var_supply(S0, S),
> +	map__to_assoc_list(N0, NList0),
> +	list__map(lambda([P0::in, P::out] is det, (
> +		P0 = V0 - Name,
> +		P = V - Name,
> +		term__coerce_var(V0, V)
> +	)), NList0, NList),
> +	map__from_assoc_list(NList, N),
> +	map__to_assoc_list(B0, BList0),
> +	list__map(lambda([Q0::in, Q::out] is det, (
> +		Q0 = V0 - Binding0,
> +		Q = V - Binding,
> +		term__coerce_var(V0, V),
> +		term__coerce(Binding0, Binding)
> +	)), BList0, BList),
> +	map__from_assoc_list(BList, B).

This code is a bit complicated -- perhaps some comments would help.

> Index: compiler/prog_data.m
...
> +	; 	inst_defn(inst_varset, inst_defn, condition) % XXX
> +	; 	mode_defn(inst_varset, mode_defn, condition) % XXX

You should explain these XXXs.

>  :- type class_method
> -	--->	pred(tvarset, existq_tvars, sym_name, list(type_and_mode),
> -			maybe(determinism), condition,
> -			class_constraints, term__context)
> +	--->	pred(tvarset, inst_varset, existq_tvars, sym_name,
> +			list(type_and_mode), maybe(determinism), condition,
> +			class_constraints, prog_context)
>  		%       VarNames, ExistentiallyQuantifiedTypeVars,
>  		%	PredName, ArgTypes, Determinism, Cond
>  		%	ClassContext, Context

Here you added a new field but you didn't update the comment.

> -	; 	func(tvarset, existq_tvars, sym_name, list(type_and_mode),
> -			type_and_mode,
> +	; 	func(tvarset, inst_varset, existq_tvars, sym_name,
> +			list(type_and_mode), type_and_mode,
>  			maybe(determinism), condition,
> -			class_constraints, term__context)
> +			class_constraints, prog_context)
>  		%       VarNames, ExistentiallyQuantfiedTypeVars,
>  		%	PredName, ArgTypes, ReturnType,
>  		%	Determinism, Cond
>  		%	ClassContext, Context

Likewise.

> +:- type prog_var_type	--->	prog_var_type.
> +:- type prog_var	==	var(prog_var_type).
> +:- type prog_varset	==	varset(prog_var_type).
> +:- type prog_substitution ==	substitution(prog_var_type).
> +:- type prog_term	==	term(prog_var_type).
> +:- type prog_context	==	term__context.
>  :- type goals		==	list(goal).
> -:- type vars		==	list(var).
> +:- type prog_vars	==	list(prog_var).

A comment here would be helpful.

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