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

[Julien Fischer]

On Wed, 16 Jul 2003, Fergus Henderson wrote:

> 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.
>
Fair enough, I've removed the state variables.

> > +++ 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.
>
I've renamed the latter one "reaching_2".


> >  :- 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".
>
I've changed it back.

> >  :- 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.
>
Also changed.


> > +					io__write_string(StdErr, "reduce reduce conflict involving:\n\t",
>
> That line should be  wrapped to fit in 80 columns.
>
Fixed.

interdiff follows:


diff -u grammar.m grammar.m
--- grammar.m	15 Jul 2003 07:56:39 -0000
+++ grammar.m	16 Jul 2003 05:40:21 -0000
@@ -366,10 +366,10 @@
 :- pred compute_first0(grammar, grammar).
 :- mode compute_first0(in, out) is det.

-compute_first0(!Grammar) :-
-	!.Grammar = grammar(Rules, Clauses, Xfs, Nont, Index, _, Follow),
+compute_first0(Grammar0, Grammar) :-
+	Grammar0 = 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).

 :- type first_stuff
 	--->	stuff(
diff -u lalr.m lalr.m
--- lalr.m	15 Jul 2003 08:32:47 -0000
+++ lalr.m	16 Jul 2003 05:43:29 -0000
@@ -73,14 +73,14 @@
 	map__lookup(Productions, ProdNum, Prod),
 	Prod = rule(NonTerminal, _Head, Symbols, _, _, _V, _C),
 	array__max(Symbols, PMax),
-	reaching(0, PMax, Symbols, First, NonTerminal, !Change, !Reaching),
+	reaching_2(0, PMax, Symbols, First, NonTerminal, !Change, !Reaching),
 	reaching(ProdNums, Productions, First, !.Change, !Reaching).

-:- pred reaching(int, int, symbols, first, nonterminal, bool, bool,
+:- pred reaching_2(int, int, symbols, first, nonterminal, bool, bool,
 		reaching, reaching).
-:- mode reaching(in, in, in, in, in, in, out, in, out) is det.
+:- mode reaching_2(in, in, in, in, in, in, out, in, out) is det.

-reaching(SN, Max, Symbols, First, C, !Change, !Reaching) :-
+reaching_2(SN, Max, Symbols, First, C, !Change, !Reaching) :-
 	( SN > Max ->
 		true
 	;
@@ -99,7 +99,7 @@
 			),
 			map__lookup(First, A, FirstA),
 			( set__member(epsilon, FirstA) ->
-				reaching(SN + 1, Max, Symbols, First, C,
+				reaching_2(SN + 1, Max, Symbols, First, C,
 					!Change, !Reaching)
 			;
 				true
diff -u mercury_syntax.m mercury_syntax.m
--- mercury_syntax.m	15 Jul 2003 08:18:15 -0000
+++ mercury_syntax.m	16 Jul 2003 05:36:15 -0000
@@ -101,38 +101,38 @@
 		io__state, io__state).
 :- mode read_module(in, in, out, di, uo) is det.

-read_module(Module, Errors, !:Result, !IO) :-
-	read_element(!:Result, !IO),
+read_module(Module, Errors, Result, !IO) :-
+	read_element(Result0, !IO),
 	(
-		!.Result = eof,
-		!:Result = module(Module, Errors)
+		Result0 = eof,
+		Result = module(Module, Errors)
 	;
-		!.Result = element(Element),
-		read_module([Element | Module], Errors, !:Result, !IO)
+		Result0 = element(Element),
+		read_module([Element | Module], Errors, Result, !IO)
 	;
-		!.Result = error(Msg, Line),
-		read_module(Module, [error(Msg, Line) | Errors], !:Result, !IO)
+		Result0 = error(Msg, Line),
+		read_module(Module, [error(Msg, Line) | Errors], Result, !IO)
 	).

 :- pred read_element(element_result, io__state, io__state).
 :- mode read_element(out, di, uo) is det.

-read_element(!:Result, !IO) :-
-	read_term(!:Result, !IO),
+read_element(Result, !IO) :-
+	read_term(Result0, !IO),
 	(
-		!.Result = eof,
-		!:Result = eof
+		Result0 = eof,
+		Result = eof
 	;
-		!.Result = error(Msg, Line),
-		!:Result = error(Msg, Line)
+		Result0 = error(Msg, Line),
+		Result = error(Msg, Line)
 	;
-		!.Result = term(VarSet, Term),
+		Result0 = term(VarSet, Term),
 		( classify(Term, VarSet, Element0) ->
 			Element = Element0
 		;
 			Element = misc(Term, VarSet)
 		),
-		!:Result = element(Element)
+		Result = element(Element)
 	).

 :- pred classify(term, varset, element).
diff -u moose.m moose.m
--- moose.m	15 Jul 2003 08:54:46 -0000
+++ moose.m	16 Jul 2003 06:17:12 -0000
@@ -220,7 +220,8 @@
 						"shift shift error.\n", !IO)
 				;
 					Error = reducereduce(R0, R1),
-					io__write_string(StdErr, "reduce reduce conflict involving:\n\t",
+					io__write_string(StdErr,
+					"reduce reduce conflict involving:\n\t",
 						!IO),
 					write_rule(StdErr, R0, Rules, !IO),
 					io__write_string(StdErr, "\t", !IO),

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