[m-dev.] for review: --use-local-vars

Fergus Henderson fjh at cs.mu.OZ.AU
Thu Mar 8 18:28:18 AEDT 2001


The log message looks good.

On 08-Mar-2001, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> Index: compiler/exprn_aux.m
> @@ -19,23 +19,20 @@
>  			bool	% --unboxed-float
>  		).
>  
> -:- pred exprn_aux__init_exprn_opts(option_table, exprn_opts).
> -:- mode exprn_aux__init_exprn_opts(in, out) is det.
> +:- pred exprn_aux__init_exprn_opts(option_table::in, exprn_opts::out) is det.
>
>  	% Determine whether an rval_const can be used as the initializer
>  	% of a C static constant.
> -:- pred exprn_aux__const_is_constant(rval_const, exprn_opts, bool).
> -:- mode exprn_aux__const_is_constant(in, in, out) is det.
> +:- pred exprn_aux__const_is_constant(rval_const::in, exprn_opts::in, bool::out)
> +	is det.

Please don't combine those kinds of reformatting changes with
other changes; it makes your changes very hard to review.
In particular it makes stuff like the following hard to spot:

> @@ -45,52 +42,48 @@
> -:- pred exprn_aux__substitute_lval_in_lval(lval, lval, lval, lval).
> -:- mode exprn_aux__substitute_lval_in_lval(in, in, in, out) is det.
> +:- pred exprn_aux__substitute_lval_in_instr(lval::in, lval::in,
> +	instruction::in, instruction::out, int::in, int::out) is det.

The meaning of the two new `int' parameters should be documented.

> @@ -373,172 +366,428 @@
>  
>  %------------------------------------------------------------------------------%
>  
> +:- pred exprn_aux__substitute_lval_in_lval(lval::in, lval::in,
> +	lval::in, lval::out, int::in, int::out) is det.
> +
> +exprn_aux__substitute_lval_in_lval(OldLval, NewLval, Lval0, Lval) :-
> +	exprn_aux__substitute_lval_in_lval(OldLval, NewLval, Lval0, Lval,
> +		0, _).

The arity of the pred declaration there doesn't match
the arity of the clause that follows.  Probably the pred
declaration should be moved so that it precedes the
clauses for the right predicate.  Also it would probably be
better to name the /6 version with a _2 suffix rather than
using arity overloading.

> +:- pred exprn_aux__substitute_lval_in_rval(lval::in, lval::in,
> +	rval::in, rval::out, int::in, int::out) is det.
> +
>  exprn_aux__substitute_lval_in_rval(OldLval, NewLval, Rval0, Rval) :-
> +	exprn_aux__substitute_lval_in_rval(OldLval, NewLval, Rval0, Rval,
> +		0, _).

Likewise.

[to be continued]

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



More information about the developers mailing list