[m-dev.] for review: update state transformation

[Fergus Henderson]

On 29-Mar-2000, Peter Ross <petdr at cs.mu.OZ.AU> wrote:
> Index: compiler/accumulator.m
...
> 	%
> 	% For each member of the assoc set determine the substitutions
> 	% needed, and also check the efficiency of the procedure isn't
> 	% worsened by swapping reordering the arguments to a call.
> 	%

s/swapping reordering/reordering/

> process_assoc_set([Id | Ids], GS, OutPrime, ModuleInfo, Substs0, Types0,
> 		Substs, Types, CS, Warnings) :-
...
> 		% ONLY swap the order of the variables if the goal is
> 		% associative.
> 	(
> 		IsCommutative = yes,
> 		CSGoal = Goal - InstMap,
> 		Warning = []
> 	;
> 		IsCommutative = no,

Is that comment correct?
I had difficulty relating the comment to the code;
the comment talks about associativity, but the code is
testing commutativity.

> :- pred heuristic(module_name::in, string::in, arity::in, prog_vars::in,
> 		set(prog_var)::out) is semidet.
> 
> heuristic(unqualified("list"), "append", 3, [_Typeinfo, A, _B, _C], Set) :-
> 	set__list_to_set([A], Set).

You should document the semantics of this predicate.

> 
> 		% inefficient order so that accumulators can be
> 		% introduced.
> 	append(StateOutputVars0, [StateOutputVar], StateOutputVars),
> 	append(Accs0, [Acc], Accs),
> 	append(BasePairs0, [StateOutputVar - Acc0], BasePairs).

That comment was a bit cryptic for me -- could you change it
to explain things in a little more detail?

compiler/assert.m:
>  assertion__is_associativity_assertion(AssertId, Module, CallVars,
> -		AssociativeVars) :-
> +		AssociativeVars, OutputVar) :-
...
> +		% There may or may not be a some [] depending on whether
> +		% the user explicity quatified the call or not.

s/quatified/qualified/

> +process_one_side(Goals, UniversiallyQuantifiedVars, PredId,
> +		LinkingVar, Vars, VarsA) :-
> +	process_two_linked_calls(Goals, UniversiallyQuantifiedVars, PredId,
> +			LinkingVar, Vars0, VarsA),
> +
> +		% Filter out all the invariant arguments, and then make
> +		% sure that their is only 3 arguments left.

s/their is/there are/

> +update(PCalls, QCalls, UniversiallyQuantifiedVars, CallVars, StateA - StateB) :-
...
> +		% If you read the predicate documentation, you will note
> +		% that for each pair of variables on the left hand side
> +		% their is an equivalent pair of variables on the right
> +		% hand side.

s/their/there/

> +++ goal_store.m	Tue Jan 11 15:44:56 2000
> @@ -0,0 +1,114 @@
> +% Module:	goal_store
> +% Main authors: petdr
> +%
> +% Given a hlds_goal store it in a goal_structure.

Did you mean goal_store rather than goal_structure?

In any case, it would be very helpful to explain what the purpose
storing a goal in a goal_store/goal_struct is.

> +++ library/list.m	2000/03/14 00:08:00
> @@ -81,6 +81,11 @@
>  		( some [BC]
>  			(list__append(B, C, BC), list__append(A, BC, ABC)) )
>  	).
> +	% construction equivalence law.
> +	% XXX when we implement rewrite rules change this law to a
> +	% rewrite rule.
> +:- promise all [L,H,T] ( append([H], T, L) <=> L = [H|T] ).

I suggest changing the wording of that slightly:
s/rules change/rules, we should change/

That completes my review; apart from the comments above and in the earlier
messages that I posted, this change looks fine.

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