[m-dev.] for review: reordering for existential types [2]

[Fergus Henderson]

On 30-Jun-1999, David Glen JEFFERY <dgj at cs.mu.OZ.AU> wrote:
> 
> > -:- pred polymorphism__process_pred(pred_id, module_info, module_info,
> > +:- pred polymorphism__maybe_process_pred(pred_id, module_info, module_info,
> >  			io__state, io__state).
> > -:- mode polymorphism__process_pred(in, in, out, di, uo) is det.
> > +:- mode polymorphism__maybe_process_pred(in, in, out, di, uo) is det.
> 
> Why has the `maybe' been introduced here? It is hard to tell from the diff.

I split the existing polymorphism__process_pred into two parts.
The first part, `maybe_process_pred', checks whether the pred needs
to be processed, and if so, it calls `process_pred', otherwise
it just calls copy_clauses_to_proc.

I can add some documentation to the source code if you want, but I
think it should be reasonably clear if you look at the source code
rather than the diff.

> > +polymorphism__process_pred(PredId, ModuleInfo0, ModuleInfo) -->
> > +	{ module_info_pred_info(ModuleInfo0, PredId, PredInfo0) },
> > +
> > +	write_pred_progress_message("% Transforming polymorphism for ",
> > +					PredId, ModuleInfo0),
> > +
> 
> Is that meant to be there (in the checked-in version)? (...or does the
> write_pred_progress_message stuff only get output with -V?)

Yes, write_pred_progress_message stuff only gets output with `-V'.

> > +polymorphism__process_proc(ProcId, ProcInfo0, PredInfo, ClausesInfo,
> > +			ExtraArgModes, ProcInfo) :-
> > +	%
> > +	% copy all the information from the clauses_info into the proc_info
> > +	%
> > +	(
> > +		( pred_info_is_imported(PredInfo)
> > +		; pred_info_is_pseudo_imported(PredInfo),
> > +		  hlds_pred__in_in_unification_proc_id(ProcId)
> >  		)
> > +	->
> > +		% XXX is this right?
> > +		ProcInfo1 = ProcInfo0
> 
> Looks right to me... but it should be easy enough to test.

Unfortunately it turned out not to be right ;-)
It is fixed in one of my later changes.

> > +polymorphism__process_unify_functor(X0, ConsId0, ArgVars0, Mode0,
> > +		Unification0, UnifyContext, GoalInfo0, Goal,
> > +		PolyInfo0, PolyInfo) :-
> 
> >  	(
> > -		{ polymorphism__no_type_info_builtin(PredModule,
> > -			PredName, PredArity)  }
> 
> Where does this case get handled now? (I see a call to
> polymorphism__no_type_info_builtin in part of the diff that I've deleted,
> but I can't tell from the context what it is doing...).

Yes, the diff for that part was very difficult to follow.
I moved some of the code around, and `diff' did a rather bad job
of showing the differences.  That call to polymorphism__no_type_info_builtin
was in the clause of polymorphism__process_goal_expr that handles
the `pragma_c_code' case.  I didn't change that clause at all, I think,
but I moved it (so that it would be near the clause for handling calls,
since the two are quite similar), and diff got rather confused.

> > +		%
> > +		% now process it
> > +		%
> > +		%polymorphism__process_goal_expr(HOCall, GoalInfo0, Goal,
> > +		%	PolyInfo0, PolyInfo)
> 
> Why is this commented out/why is this comment here?

That's a good question.  I will change the comment to explain things properly.
See the diff below.

> > +	% If it turns out that the predicate was non-polymorphic,
> > +	% lambda.m will (I hope) turn the lambda expression
> > +	% back into a higher-order pred constant again.
> 
> That sounds like the right way to handle it, but hoping probably isn't the
> best thing to do...

I've confirmed that this does indeed happen (for at least one test case).
I will delete the "(I hope)" from the comment.

> > +			error("modecheck_unification: list__drop failed")
> 
> modecheck_unification isn't the predname any more.

Fixed.  See the diff below.

> > +			error("Sorry, not implemented: determinism inference for higher-order predicate terms")
> 
> Ouch!

Yes, that is nasty, but that is something that was already a problem
for the existing compiler, not anything new in the existential types
branch; all I did was move that code from one place (modecheck_unify.m)
to another (polymorphism.m).  So I won't fix that one as part of this
change.

> > +% this is duplicated in modecheck_unify.m
> > +:- pred make_fresh_vars(list(type), prog_varset, map(prog_var, type),
> > +			list(prog_var), prog_varset, map(prog_var, type)).
> > +:- mode make_fresh_vars(in, in, in, out, out, out) is det.
> 
> At the least you should XXX that. (Have I just verbed a new noun?)

Thanks to Simon Taylor's prompting, I fixed that code duplication in one
of my later changes.

----------

Estimated hours taken: 0.25

compiler/polymorphism.m:
	Change a few comments to clarify things raised by dgj's review.

Workspace: /home/mercury0/fjh/mercury-other
Index: compiler/polymorphism.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/polymorphism.m,v
retrieving revision 1.163.2.8
diff -u -r1.163.2.8 polymorphism.m
--- polymorphism.m	1999/06/29 18:26:40	1.163.2.8
+++ polymorphism.m	1999/06/29 18:42:37
@@ -1235,11 +1235,19 @@
 		HOCall = higher_order_call(FuncVar, ArgVars, ArgTypes,
 			Modes, Det, function),
 
+		/*******
 		%
-		% now process it
+		% Currently we don't support higher-order polymorphism;
+		% all closures are monomorphic (any type_infos needed are
+		% supplied when the closure is created, not when it is called).
+		% Therefore we don't need to bother recursively processing
+		% the higher-order function call.  If we were to ever add
+		% support for higher-order polymorphism, then we would need
+		% to uncomment this code.
 		%
-		%polymorphism__process_goal_expr(HOCall, GoalInfo0, Goal,
-		%	PolyInfo0, PolyInfo)
+		polymorphism__process_goal_expr(HOCall, GoalInfo0, Goal,
+			PolyInfo0, PolyInfo)
+		********/
 		Goal = HOCall - GoalInfo0,
 		PolyInfo = PolyInfo0
 	;
@@ -1318,8 +1326,8 @@
 	% Secondly, this pass (polymorphism.m) is a lot easier
 	% if we don't have to handle higher-order pred consts.
 	% If it turns out that the predicate was non-polymorphic,
-	% lambda.m will (I hope) turn the lambda expression
-	% back into a higher-order pred constant again.
+	% lambda.m will turn the lambda expression back into a
+	% higher-order pred constant again.
 	%
 	% Note that this transformation is also done by modecheck_unify.m,
 	% in case we are rerunning mode analysis after lambda.m has already
@@ -1452,7 +1460,7 @@
 	( list__drop(NumArgModes - NumLambdaVars, ArgModes, LambdaModes0) ->
 		LambdaModes = LambdaModes0
 	;
-		error("modecheck_unification: list__drop failed")
+		error("convert_pred_to_lambda_goal: list__drop failed")
 	),
 	proc_info_declared_determinism(ProcInfo, MaybeDet),
 	( MaybeDet = yes(Det) ->

-- 
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.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------