[m-rev.] for review: more changes to moose

[Fergus Henderson]

On 15-Jul-2003, Julien Fischer <juliensf at students.cs.mu.OZ.AU> wrote:
> 
> -compute_first0(Grammar0, Grammar) :-
> -	Grammar0 = grammar(Rules, Clauses, Xfs, Nont, Index, _, Follow),
> +compute_first0(!Grammar) :-
> +	!.Grammar = grammar(Rules, Clauses, Xfs, Nont, Index, _, Follow),
>  	compute_first(Rules, First),
> -	Grammar = grammar(Rules, Clauses, Xfs, Nont, Index, First, Follow).
> +	!:Grammar = grammar(Rules, Clauses, Xfs, Nont, Index, First, Follow).

I'm a bit dubious about use of state variables actually improving
readability in cases like this where there are only the initial and
final states, with no intermediate states.  However, I suppose it
doesn't matter too much either way.

> +++ lalr.m	15 Jul 2003 08:32:47 -0000
> @@ -65,50 +65,44 @@
>  :- pred reaching(list(prodnum), rules, first, bool, reaching, reaching).
>  :- mode reaching(in, in, in, in, in, out) is det.
> 
> -reaching([], _Productions, _First, no, Reaching, Reaching).
> -reaching([], Productions, First, yes, Reaching0, Reaching) :-
> +reaching([], _Productions, _First, no, !Reaching).
> +reaching([], Productions, First, yes, !Reaching) :-
>  	prodnums(Productions, ProdNums),
> -	reaching(ProdNums, Productions, First, no, Reaching0, Reaching).
> -reaching([ProdNum|ProdNums], Productions, First, Ch0, Reaching0, Reaching) :-
> -	lookup(Productions, ProdNum, Prod),
> +	reaching(ProdNums, Productions, First, no, !Reaching).
> +reaching([ProdNum|ProdNums], Productions, First, !.Change, !Reaching) :-
> +	map__lookup(Productions, ProdNum, Prod),
>  	Prod = rule(NonTerminal, _Head, Symbols, _, _, _V, _C),
>  	array__max(Symbols, PMax),
> -	reaching(0, PMax, Symbols, First, NonTerminal, Ch0, Ch1,
> -		Reaching0, Reaching1),
> -	reaching(ProdNums, Productions, First, Ch1, Reaching1, Reaching).
> +	reaching(0, PMax, Symbols, First, NonTerminal, !Change, !Reaching),
> +	reaching(ProdNums, Productions, First, !.Change, !Reaching).
> 
>  :- pred reaching(int, int, symbols, first, nonterminal, bool, bool,
>  		reaching, reaching).
>  :- mode reaching(in, in, in, in, in, in, out, in, out) is det.
> 
> -reaching(SN, Max, Symbols, First, C, Ch0, Ch, Reaching0, Reaching) :-
> +reaching(SN, Max, Symbols, First, C, !Change, !Reaching) :-

Combining arity overloading with state variable notation is a bad idea, IMHO.

So I suggest that one of these predicates named "reaching" should be
renamed.


>  :- pred read_module(module, list(module_error), module_result,
>  		io__state, io__state).
>  :- mode read_module(in, in, out, di, uo) is det.
> 
> -read_module(Module0, Errors0, Result) -->
> -	read_element(Result0),
> +read_module(Module, Errors, !:Result, !IO) :-
> +	read_element(!:Result, !IO),
>  	(
> -		{ Result0 = eof },
> -		{ Result = module(Module0, Errors0) }
> +		!.Result = eof,
> +		!:Result = module(Module, Errors)
>  	;
> -		{ Result0 = element(Element) },
> -		read_module([Element|Module0], Errors0, Result)
> +		!.Result = element(Element),
> +		read_module([Element | Module], Errors, !:Result, !IO)
>  	;
> -		{ Result0 = error(Msg, Line) },
> -		read_module(Module0, [error(Msg, Line)|Errors0], Result)
> +		!.Result = error(Msg, Line),
> +		read_module(Module, [error(Msg, Line) | Errors], !:Result, !IO)
>  	).

Here I am dubious about the merits of using state variable syntax
for "Result".

>  :- pred read_element(element_result, io__state, io__state).
>  :- mode read_element(out, di, uo) is det.
> 
> -read_element(Result) -->
> -	read_term(Result0),
> +read_element(!:Result, !IO) :-
> +	read_term(!:Result, !IO),
>  	(
> -		{ Result0 = eof },
> -		{ Result = eof }
> +		!.Result = eof,
> +		!:Result = eof
>  	;
> -		{ Result0 = error(Msg, Line) },
> -		{ Result = error(Msg, Line) }
> +		!.Result = error(Msg, Line),
> +		!:Result = error(Msg, Line)
>  	;
> -		{ Result0 = term(VarSet, Term) },
> -		( { classify(Term, VarSet, Element0) } ->
> -			{ Element = Element0 }
> +		!.Result = term(VarSet, Term),
> +		( classify(Term, VarSet, Element0) ->
> +			Element = Element0
>  		;
> -			{ Element = misc(Term, VarSet) }
> +			Element = misc(Term, VarSet)
>  		),
> -		{ Result = element(Element) }
> +		!:Result = element(Element)
>  	).

Likewise.

> +					io__write_string(StdErr, "reduce reduce conflict involving:\n\t",

That line should be  wrapped to fit in 80 columns.

Otherwise that looks fine.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------