[m-rev.] Re: for review: default modes for higher-order func insts

[David Overton]

On Fri, Aug 03, 2001 at 09:44:37PM +1000, Fergus Henderson wrote:
> On 03-Aug-2001, David Overton <dmo at cs.mu.OZ.AU> wrote:
> > Implement a change to the mode system suggested by Ralph Becket to make use of
> > higher order functions a bit easier.
> 
> There should really be some documentation of this in the Mercury language
> reference manual.

Documentation is still to come.  I would like to get this committed
though, so that people like Ralph and Michael who have been asking for
it can try it out.

> 
> The most difficult part of this diff to review is all of the places that
> you *didn't* change.  There's a high likelihood of some existing code
> not doing the right thing for the new semantics of `ground'.
> 
> However, I have done my best.  One thing I noticed is that the code
> for propagating higher-order types into insts in mode_util.m isn't
> needed anymore, and should be deleted.

I will make this a separate change.

> But because of the difficulty
> in reviewing, the change will need careful testing.
> 
> > compiler/inst_match.m:
> > 	In 'inst_matches_initial' and 'inst_matches_final', ensure that higher
> > 	order inst information is not lost from non-standard function insts.
> > 	Also allow 'inst_matches_{initial,final,binding}' to succeed
> > 	where the first inst is a standard function inst and the
> > 	second is ground.
> 
> I think you should also add some comments to inst_is_ground explaining
> what it does for non-standard function insts, and why.  Currently it
> succeeds, which I think is the right thing for most callers, but not for
> all.  There is at least one place where this issue with inst_is_ground
> leads to a bug with your current diff because of somewhere that
> you didn't change.

done.

> 
> You need go through every call to inst_is_ground and see whether it
> is OK for each call to succeed for non-standard function insts.  This will
> require searching up the call graph in some cases (e.g. you'll need to
> look at all callers of mode_is_fully_input and mode_is_fully_output).

done.

> 
> > +++ compiler/inst_util.m	2 Aug 2001 15:05:03 -0000
> > @@ -98,6 +98,25 @@
> >  	%       the same in both.
> >  
> >  %-----------------------------------------------------------------------------%
> > +
> > +	% Succeed iff the first argument is a function pred_inst_info
> > +	% with non-standard mode.
> > +
> > +:- pred pred_inst_info_is_nonstandard_func_mode(pred_inst_info, module_info).
> > +:- mode pred_inst_info_is_nonstandard_func_mode(in, in) is semidet.
> 
> s/with non-standard mode/whose mode does not match the standard func mode/
> 
> e.g. this predicate fails for "func(in) = uo", because that matches
> the standard func mode "func(in) = out", even though it isn't the same
> as the standard func mode.
> 
> (Might be worth adding the preceding sentence as a comment.)

done.

> 
> > +	% Succeed iff the first argument is a function ground_inst_info
> > +	% with non-standard mode.
> > +
> > +:- pred ground_inst_info_is_nonstandard_func_mode(ground_inst_info,
> > +			module_info).
> > +:- mode ground_inst_info_is_nonstandard_func_mode(in, in) is semidet.
> 
> Likewise.

done.

> 
> > +++ tests/hard_coded/ho_func_default_inst.m	3 Aug 2001 00:17:04 -0000
> > @@ -0,0 +1,43 @@
> > +% This test checks that a higher order func type with inst ground is 
> > +% able to be treated as a though it has the default function mode.
> 
> I think this change needs more test cases.
> The reasoning involved in handling some of the tricky cases is
> quite easy to get wrong, and so even if your change gets it
> right, it's important to have test cases in the test suite
> to make sure that future maintainers don't mess it up.
> 
> In particular, I think it's important to test matching
> of the following modes against `in':
> 
> 	- modes like `func(in) = uo)'
> 	- modes like `func(di) = uo)'
> 	- modes like `func(in) = out(non_empty_list)'
> 	- modes like `func(in(non_empty_list)) = out'
> 

done.

> 
> Apart from that, this diff looks fine.  Please post another diff
> (preferably relative) when you've addressed these comments.

Here it is.


Estimated hours taken: 8
Branches: main

Implement a change to the mode system suggested by Ralph Becket to make use of
higher order functions a bit easier.

During mode checking of higher order calls, if the variable being called has a
higher-order function type, but only a ground inst with no higher-order
information, assume that it has the default function modes.

Also, when doing anything that might cause a variable's inst to lose higher
order mode information, report a mode error if the variable has a non-standard
higher order function mode.  Situations where this may occur are at call sites,
exit sites and when merging insts at the end of a branched goal.

Note that because of this restriction, this change is not backwards compatible.

compiler/inst_util.m:
	Define some predicates to check for and produce pred_inst_infos for
	default function modes.
	In 'inst_merge', ensure that higher order inst information is not lost
	from non-standard function insts.

compiler/inst_match.m:
	In 'inst_matches_initial' and 'inst_matches_final', ensure that higher
	order inst information is not lost from non-standard function insts.
	Also allow 'inst_matches_{initial,final,binding}' to succeed
	where the first inst is a standard function inst and the
	second is ground.

compiler/modecheck_call.m:
	In 'modecheck_higher_order_call', if the variable to be called has no
	pred_inst_info, but the correct higher-order function type, assume it
	has the default function modes. 

tests/hard_coded/Mmakefile:
tests/hard_coded/ho_func_default_inst.m:
tests/hard_coded/ho_func_default_inst.exp:
tests/invalid/Mmakefile:
tests/invalid/ho_default_func_1.m:
tests/invalid/ho_default_func_1.err_exp:
tests/invalid/ho_default_func_2.m:
tests/invalid/ho_default_func_2.err_exp:
tests/invalid/ho_default_func_3.m:
tests/invalid/ho_default_func_3.err_exp:
	Add some test cases.

diff -u compiler/inst_match.m compiler/inst_match.m
--- compiler/inst_match.m
+++ compiler/inst_match.m
@@ -145,12 +145,18 @@
         % succeed if the inst is fully ground (i.e. contains only
         % `ground', `bound', and `not_reached' insts, with no `free'
         % or `any' insts).
+	% This predicate succeeds for non-standard function insts so some care
+	% needs to be taken since these inst may not be replaced by a less
+	% precise inst that uses the higher-order mode information.
 :- pred inst_is_ground(module_info, inst).
 :- mode inst_is_ground(in, in) is semidet.
 
         % succeed if the inst is not partly free (i.e. contains only
         % `any', `ground', `bound', and `not_reached' insts, with no
         % `free' insts).
+	% This predicate succeeds for non-standard function insts so some care
+	% needs to be taken since these inst may not be replaced by a less
+	% precise inst that uses the higher-order mode information.
 :- pred inst_is_ground_or_any(module_info, inst).
 :- mode inst_is_ground_or_any(in, in) is semidet.
 
diff -u compiler/inst_util.m compiler/inst_util.m
--- compiler/inst_util.m
+++ compiler/inst_util.m
@@ -99,24 +99,33 @@
 
 %-----------------------------------------------------------------------------%
 
-	% Succeed iff the first argument is a function pred_inst_info
-	% with non-standard mode.
+:- pred inst_contains_nonstandard_func_mode(inst, module_info).
+:- mode inst_contains_nonstandard_func_mode(in, in) is semidet.
+
+	% inst_contains_nonstandard_func_mode(Inst, ModuleInfo) succeeds iff the
+	% inst contains a higher-order function inst that does not match the
+	% standard function mode `(in, ..., in) = out is det'.
+	% E.g. this predicate fails for "func(in) = uo" because that matches the
+	% standard func mode "func(in) = out", even though it isn't the same as
+	% the standard func mode.
 
 :- pred pred_inst_info_is_nonstandard_func_mode(pred_inst_info, module_info).
 :- mode pred_inst_info_is_nonstandard_func_mode(in, in) is semidet.
 
-	% Succeed iff the first argument is a function ground_inst_info
-	% with non-standard mode.
+	% Succeed iff the first argument is a function pred_inst_info
+	% whose mode does not match the standard func mode.
 
 :- pred ground_inst_info_is_nonstandard_func_mode(ground_inst_info,
 			module_info).
 :- mode ground_inst_info_is_nonstandard_func_mode(in, in) is semidet.
 
-
-	% Return the standard mode for a function of the given arity.
+	% Succeed iff the first argument is a function ground_inst_info
+	% whose mode does not match the standard func mode.
 
 :- func pred_inst_info_standard_func_mode(arity) = pred_inst_info.
 
+	% Return the standard mode for a function of the given arity.
+
 %-----------------------------------------------------------------------------%
 
 :- implementation.
@@ -1633,11 +1642,37 @@
 %-----------------------------------------------------------------------------%
 %-----------------------------------------------------------------------------%
 
+inst_contains_nonstandard_func_mode(Inst, ModuleInfo) :-
+	set__init(Expansions0),
+	inst_contains_nonstandard_func_mode_2(Inst, ModuleInfo, Expansions0).
+
+:- pred inst_contains_nonstandard_func_mode_2(inst, module_info, set(inst)).
+:- mode inst_contains_nonstandard_func_mode_2(in, in, in) is semidet.
+
+inst_contains_nonstandard_func_mode_2(ground(_, GroundInstInfo), ModuleInfo,
+		_Expansions) :-
+	ground_inst_info_is_nonstandard_func_mode(GroundInstInfo, ModuleInfo).
+inst_contains_nonstandard_func_mode_2(bound(_, BoundInsts), ModuleInfo,
+		Expansions) :-
+	list__member(functor(_, Insts), BoundInsts),
+	list__member(Inst, Insts),
+	inst_contains_nonstandard_func_mode_2(Inst, ModuleInfo, Expansions).
+inst_contains_nonstandard_func_mode_2(inst_var(_), _, _) :-
+	error("internal error: uninstantiated inst parameter").
+inst_contains_nonstandard_func_mode_2(Inst, ModuleInfo, Expansions0) :-
+	Inst = defined_inst(InstName),
+	\+ set__member(Inst, Expansions0),
+	set__insert(Expansions0, Inst, Expansions1),
+	inst_lookup(ModuleInfo, InstName, Inst2),
+	inst_contains_nonstandard_func_mode_2(Inst2, ModuleInfo, Expansions1).
+
+%-----------------------------------------------------------------------------%
+
 pred_inst_info_is_nonstandard_func_mode(PredInstInfo, ModuleInfo) :-
 	PredInstInfo = pred_inst_info(function, ArgModes, _),
 	Arity = list__length(ArgModes),
-	\+ pred_inst_matches(pred_inst_info_standard_func_mode(Arity),
-		PredInstInfo, ModuleInfo).
+	\+ pred_inst_matches(PredInstInfo,
+		pred_inst_info_standard_func_mode(Arity), ModuleInfo).
 
 ground_inst_info_is_nonstandard_func_mode(GroundInstInfo, ModuleInfo) :-
 	GroundInstInfo = higher_order(PredInstInfo),
diff -u tests/invalid/Mmakefile tests/invalid/Mmakefile
--- tests/invalid/Mmakefile
+++ tests/invalid/Mmakefile
@@ -47,6 +47,7 @@
 	func_errors.m \
 	funcs_as_preds.m \
 	ho_default_func_1.m \
+	ho_default_func_3.m \
 	ho_type_mode_bug.m \
 	ho_unique_error.m \
 	impure_method_impl.m \

-- 
David Overton      Department of Computer Science & Software Engineering
PhD Student        The University of Melbourne, Victoria 3010, Australia
+61 3 8344 9159    http://www.cs.mu.oz.au/~dmo
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------