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

[David Glen JEFFERY]

From: David Glen JEFFERY <dgj at murlibobo.cs.mu.OZ.AU>
To: mercury-developers at cs.mu.OZ.AU
Cc: 
Bcc: 
Subject: Re: [m-dev.] for review: add support for existential types [2/4]
Reply-To: 
In-Reply-To: <19980630115828.31256 at murlibobo.cs.mu.OZ.AU>; from Fergus Henderson on Tue, Jun 30, 1998 at 11:58:28AM +1000

On 30-Jun-1998, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> [continued from part 1]
> 
> Index: compiler/polymorphism.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/polymorphism.m,v
> retrieving revision 1.138
> diff -u -r1.138 polymorphism.m
> --- polymorphism.m	1998/06/19 00:42:37	1.138
> +++ polymorphism.m	1998/06/29 09:26:10
>
> @@ -375,12 +385,12 @@
>  
>  %	It is misleading to output this message for predicates which are
>  %	not defined in this module, and we get far too many of them anyway.
> -%	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... either the comment above this code should be
deleted (if it no longer holds), or the call to write_proc_progress_message
should not be uncommented. (How's that for a double negative? ;-) ).

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

>  	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

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

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

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

> +% 
> +% 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).

[That's the second part of four... more to come].


love and cuddles,
dgj
-- 
David Jeffery (dgj at cs.mu.oz.au) |  Marge: Did you just call everyone "chicken"?
PhD student,                    |  Homer: Noooo.  I swear on this Bible!
Department of Computer Science  |  Marge: That's not a Bible; that's a book of
University of Melbourne         |         carpet samples!
Australia                       |  Homer: Ooooh... Fuzzy.