[m-dev.] for review: add support for existential types [2/4]

[Fergus Henderson]

On 02-Jul-1998, David Glen JEFFERY <dgj at cs.mu.OZ.AU> wrote:
> From: David Glen JEFFERY <dgj at murlibobo.cs.mu.OZ.AU>
> > -%	write_proc_progress_message("% Transforming polymorphism for ",
> > -%				PredId, ProcId, ModuleInfo0, IO0, IO1),
> > -	IO1 = IO0,
> > +	write_proc_progress_message("% Transforming polymorphism for ",
> > +				PredId, ProcId, ModuleInfo0, IO0, IO1),
> > +%	IO1 = IO0,
> 
> This doesn't look right... 

Yes, I enabled that code for debugging, but I should have re-disabled it.
I will do so.

> > +		%
> > +		% if there were any existentially quantified type variables,
> > +		% either in this predicate or in any predicate that it calls,
> > +		% then we may need to recompute the instmap deltas too
> > +		%
> 
> Could you expand that a little please? I don't understand why the
> quantification matters.

I've added the following:

                % (The instmap deltas only need to be recomputed if we
                % change which variables are bound by the subgoals, i.e.
                % if any of the new variables that we introduced have mode
                % `out' rather than mode `in'.  This can happen only if some 
                % of the type variables are existentially quantified rather 
                % than universally quantified.)

 
> >  	in_mode(In),
> > -	list__duplicate(NumExtraVars, In, ExtraModes),
> > -	list__append(ExtraModes, ArgModes0, ArgModes),
> > +	out_mode(Out),
> > +	list__length(UnconstrainedUnivTVars, NumUnconstrainedUnivTVars),
> > +	list__length(UnconstrainedExistTVars, NumUnconstrainedExistTVars),
> > +	list__length(UnivHeadTypeClassInfoVars, NumUnivClassInfoVars),
> > +	list__length(ExistHeadTypeClassInfoVars, NumExistClassInfoVars),
> > +	list__duplicate(NumUnconstrainedUnivTVars, In, UnivTypeInfoModes),
> > +	list__duplicate(NumUnconstrainedExistTVars, Out, ExistTypeInfoModes),
> > +	list__duplicate(NumUnivClassInfoVars, In, UnivTypeClassInfoModes),
> > +	list__duplicate(NumExistClassInfoVars, Out, ExistTypeClassInfoModes),
> > +	list__condense([UnivTypeClassInfoModes, ExistTypeClassInfoModes,
> > +		UnivTypeInfoModes, ExistTypeInfoModes, ArgModes0], ArgModes),
> 
> You need to document somewhere that the parameter passing convention is now:
> 
> 	UnivTypeClassInfos, ExistTypeClassInfos, UnivTypeInfos, ExistTypeInfos
> 
> I think there is a (now obsolete) comment about this at the top of
> polymorphism.m

I didn't find any such comment.

I added the following comment at the end of the comments at the top
of polymorphism.m:

%
% The argument passing convention is that the new parameters
% introduced by this pass are placed in the following order:
%
%       First the UnivTypeClassInfos (for universally quantified constraints)
%       then the ExistTypeClassInfos (for existentially quantified constraints)
%       then the UnivTypeInfos (for universally quantified type variables)
%       then the ExistTypeInfos (for existentially quantified type variables)
%       and finally the original arguments of the predicate.
%

> > +:- pred polymorphism__assign_var_2(var, var, hlds_goal).
> > +:- mode polymorphism__assign_var_2(in, in, out) is det.
> > +
> > +polymorphism__assign_var_2(Var1, Var2, Goal) :-
> > +
> > +	% Doing just this wouldn't work, because we also need to fill in
> > +	% the mode and determinism info:
> > +	%	term__context_init(Context),
> > +	%	create_atomic_unification(Var1, var(Var2), Context, explicit,
> > +	% 		[], Goal),
> > +
> > +	Ground = ground(shared, no),
> > +	Mode = ((free -> Ground) - (Ground -> Ground)),
> > +	UnifyInfo = assign(Var1, Var2),
> > +	UnifyC = unify_context(explicit, []),
> > +	goal_info_init(GoalInfo0),
> > +	instmap_delta_from_assoc_list([Var1 - Ground], InstMapDelta),
> > +	goal_info_set_instmap_delta(GoalInfo0, InstMapDelta, GoalInfo1),
> > +	goal_info_set_determinism(GoalInfo1, det, GoalInfo2),
> > +	set__list_to_set([Var1, Var2], NonLocals),
> > +	goal_info_set_nonlocals(GoalInfo2, NonLocals, GoalInfo),
> > +	Goal = unify(Var1, var(Var2), Mode, UnifyInfo, UnifyC) - GoalInfo.
> 
> goal_info_init/4 would make this code a little simpler.

Yes, you're right.  That part now reads as follows:

	...
        UnifyInfo = assign(Var1, Var2),
        UnifyC = unify_context(explicit, []),
        set__list_to_set([Var1, Var2], NonLocals),
        instmap_delta_from_assoc_list([Var1 - Ground], InstMapDelta),
        Determinism = det,
        goal_info_init(NonLocals, InstMapDelta, Determinism, GoalInfo),
        Goal = unify(Var1, var(Var2), Mode, UnifyInfo, UnifyC) - GoalInfo.

> > +% XXX the following code ought to be rewritten to handle
> > +% existential/universal type_infos and type_class_infos
> > +% in a more consistent manner.
> 
> I've seen this comment a few times (with good justification). I think that
> this should be mentioned in the comment at the top of the file.

I've added the following comment

% XXX The way the code in this module handles existential type classes
% and type class constraints is a bit ad-hoc, in general; there are
% definitely parts of this code (marked with XXXs below) that could
% do with a rewrite to make it more consistent and hence more maintainable.

near the very start of the file.

> > +		%
> > +		% update the instmap delta for typeinfo vars and
> > +		% typeclassinfo vars for any existentially quantified
> > +		% type vars in the callee's type: such typeinfo variables
> > +		% are produced by this call
> > +		%
> 
> My comment above applies here too.

I've added the following comment after that:

                % (universally quantified typeinfo and typeclassinfo vars
                % are input to the goal, and their inst is not changed by
                % the goal, so they don't need to be mentioned in the
                % instmap delta)

> > +% 
> > +% Constraints which are already in the TypeClassInfoMap are assumed to
> > +% have already had their typeclass_infos initialized; for them, we
> > +% just return the variable in the TypeClassInfoMap.
> > +%
> 
> When can this happen? (Just out of curiosity... the comment is fine as it
> stands).

Oh, it can happen if there are calls to two different predicates
with the same type class constraints.

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