[m-dev.] for review: type specialisation round 2

[Fergus Henderson]

On 08-Sep-1998, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> --- higher_order.m	1998/08/31 02:10:53	1.3
> +++ higher_order.m	1998/09/08 00:26:27
...
> +	% Collect the list class_constraints from the list of argument types.

s/list/list of/

> +:- pred maybe_add_constraint(list(class_constraint)::in,
> +		class_constraint::in, list(class_constraint)::out) is det.
> +
> +maybe_add_constraint(Constraints0, Constraint0, Constraints) :-
> +	Constraint0 = constraint(ClassName, Types0),
> +	strip_term_contexts(Types0, Types),
> +	Constraint = constraint(ClassName, Types),
> +	(
> +		% Remove duplicates
> +		\+ list__member(Constraint, Constraints0),
> +
> +		% A constraint says nothing if the types are all ground.
> +		list__member(Type, Types),
> +		\+ term__is_ground(Type)

This is not always true.  Constraints on ground types can make sense
if the instance declaration is visible in the caller but not in the
callee.

For example, if you have an abstract type `adt' declared in the
interface of one module

	:- module foo.
	:- interface
	:- class c where [...].
	:- type adt.

	:- implementation.
	:- instance c(adt) where ...

	foo:p :-
		bar:q.

then in a different module you could have

	:- module bar.
	:- interface.
	:- import_module foo.
	:- pred q <= c(int).
	q :- ...

and here the ground constraint `c(int)' makes sense, since the instance
declaration isn't visible inside `bar', it's only visible from the caller
in the implementation of module `foo'.

> +:- pred add_extra_typeclass_infos_2(list(var)::in, list(type)::in,
> +		map(class_constraint, var)::in,
> +		map(class_constraint, var)::out) is semidet.
> +
> +add_extra_typeclass_infos_2([], [], TCVarMap, TCVarMap).
> +add_extra_typeclass_infos_2([Var | Vars], [Type0 | Types],
> +		TCVarMap0, TCVarMap) :-
> +	strip_term_context(Type0, Type),
> +	(
> +		polymorphism__typeclass_info_class_constraint(Type,
> +			Constraint),
> +		Constraint = constraint(_, ConstrainedTypes),
> +
> +		(
> +			list__member(ConstrainedType, ConstrainedTypes),
> +			\+ term__is_ground(ConstrainedType)
> +		->
> +			true
> +		;
> +			fail
> +		)

Hmm, `(A -> true ; fail)' is the same as `A'.
I don't think the if-then-else improves readability here, does it?

> +                "--no-disable-opt-for-trace",
> +                "\tEnabling tracing usually disables optimizations which could",
> +                "\tmake it difficult to relate the trace to the source code.",
> +                "\tThis option is useful when debugging those optimizations."

The help message here says what the option is useful for, but it
doesn't actually say what the option _does_.

(Ditto for the corresponding change to user_guide.texi.)

> Index: type_util.m
...
>  :- mode apply_rec_substitution_to_type_map(in, in, out) is det.
>  
> -        % Update a map from tvar to type_info_locn, using the type substititon
> -        % to rename tvars and a variable substition to rename vars.
> +        % Update a map from tvar to type_info_locn, using the type renaming
> +        % and substitution to rename tvars and a variable substition to
> +        % rename vars.

s/substition/substitution/

If there is both a type renaming and a type substitution, then
you should say in which order these are applied (since it makes
a difference).

>          % If tvar maps to a another type variable, we keep the new
>          % variable, if it maps to a type, we remove it from the map.
>  
>  :- pred apply_substitutions_to_var_map(map(tvar, type_info_locn), tsubst,
> -        map(var, var), map(tvar, type_info_locn)).
> -:- mode apply_substitutions_to_var_map(in, in, in, out) is det.
> +        map(tvar, type), map(var, var), map(tvar, type_info_locn)).
> +:- mode apply_substitutions_to_var_map(in, in, in, in, out) is det.
> +
> +        % Update a map from class_constraint to var, using the type renaming
> +        % and substitution to rename tvars and a variable substition to
> +        % rename vars.

s/substition/substitution/

> +        % If the constraint no longer constraints any type variables,
> +        % we remove it from the map.

s/constraints/constrains/

See also my above comment about ground constraints making sense.
Same point applies here.

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