[m-dev.] for review: accumulator.m

[Fergus Henderson]

On 25-Jun-1998, Peter David ROSS <petdr at cs.mu.OZ.AU> wrote:
> % File:		accumulator.m
> % Main authors: petdr
> %
> % Identify procedures which would become tail recursive if some of the
> % output variables are accumulated and then transform them to the tail
> % recursive form.
> %
> % The code looks for procedures which are in the following form (Y is a 
> % set of variables).
> %
> % :- pred p(T::in, U::out) is XXX.

Please use "XXX" only as a "red flag" to mark potential problems,
things that don't work quite right, etc., not as a
general placeholder variable.  Just use something like "Blah" or
"Foo" or "Whatever".

> % p(X,Y) :-
> % 	minimal(X),
> % 	identity(Y).
> % p(X,Y) :-
> % 	decompose(X, Xh, Xr),
> % 	p(Xr, Y0),
> % 	compose(Xh, Y0, Y).
> %
> % Then as long as the compose section of the predicate is assocative
> % (the difficult bit) then the code can be transformed into
> %
> % p(X,Y) :-

s/X,Y/X, Y/g

If each of these variables could actually be a set of variables
then I suggest it would be clearer to name them Xs, Ys, etc.

> % XXX	In the future this phase should transform procedures for which
> %	only construction unifications remain after p' inside p'.  As

Do you mean "... should also transform ...",
or "... should only transform ..."?

> %	dmo's last call modulo construction algorithm will complete the
> %	transformation into a tail recursive procedure.

It's probably more useful to say "the last call modulo constructor application
optimization (in lco.m) will ...".  It's far more useful to tell the reader
which file it is in rather than who the author was.

> 	%
> 	% Type to reduce the number of arguments to some predicates.
> 	%
> :- type info
> 	--->	info(
> 			proc_id,
> 			proc_info,
> 			pred_id,
> 			pred_info,
> 			module_info
> 		).

As I said last time you should use a more informative name than `info'.

The comment here is quite mysterious and doesn't really explain things
very well at all.  Normally a type doesn't *do* anything, it just holds
information.

> :- type	acc_info
> 	--->	simple(
> 			list(acc_var),
> 			sym_name,		% of the introduced pred
> 			pred_id,		% of the introduced pred
> 			proc_id			% of the introduced pred
> 		).

You should document the purpose of this type.

> 	%
> 	% This type is threaded through the processing of the compose
> 	% goals.  It's purpose is to help identify whether or not the
> 	% goals as a whole are assocative.

Peter, please please *please* go and read a grammar book about the
use of apostrophes.

With personal pronouns such as "it", you should use an apostrophe
*only* if it is a contraction of two words.  That is, you can abbreviate
"it is" as "it's", but for personal pronouns you should never use an
appostrophe to indicate possesion.

> 	% Static vars are those vars which will hold the same value in a
> 	% recursive call no matter which direction the calls are
> 	% processed in. ie the Head of the list will always be the same.
> 	%
> 	% Dynamic vars are those vars which will hold different values. ie
> 	% the running sum of the elements in a list will be different
> 	% depending on which end you start summing from.

s/ie/e.g./g

s/Head/head/

I don't really understand the definition of static variables;
perhaps you could choose a better example?

> 	%
> 	% Indicator of whether a call stayed associative (ie +) or
> 	% became right associative (ie append) during the transform.
> 	%

s/ie/e.g./g

s/transform/transformation/

I still don't understand the distinction between associative and right
associative.

> :- type var_info
> 	--->	var_info(
> 			var,
> 			int			% position of var in the arg 
> 						% list for a call.
> 		).

Do position numbers start from 0 or 1?

[To be continued...]

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