[m-dev.] for review: add code to read terms from strings

Fergus Henderson fjh at cs.mu.OZ.AU
Sun May 17 01:40:41 AEST 1998


On 15-May-1998, Mark Anthony BROWN <dougl at cs.mu.OZ.AU> wrote:
> > +% The type `posn' represents a position within a string.
> > +:- type posn
> > +	--->	posn(int, int, int).
> > +		% line number, offset for start of line, current offset
> > +		% (the first two are used only for the purposes of
> > +		% computing term_contexts, for use e.g. in error messages).
> 
> "offset for start of line" is confusing.  Maybe s/for/of/ to
> distinguish it from "offset from start of line".

Done.

> Also, do the offsets start from zero or one?

I've added "Offsets start from zero.".

> > +	%
> > +	% A line number directive token is `#' followed by an integer
> > +	% (specifying the line number) followed by a newline.
> > +	% Such a token sets the source line number for the next
> > +	% line, but it is otherwise ignored.  This means that line number
> > +	% directives may appear anywhere that a token may appear, including
> > +	% in the middle of terms.
> > +	% (The source file name can be set with a `:- pragma source_file' 
> > +	% declaration.)
> > +	%
> 
> This comment seems to appear twice.

Second occurrence deleted.

> > +		lexer__grab_string(String, Posn0, BinaryString),
> > +		{ lexer__conv_string_to_int(BinaryString, 8, Token) },
> 
> s/8/16/

Ouch!  Thanks for spotting that cut-and-paste bug.
Thank goodness for code reviews! ;-)

> > +:- pred parser__read_term_from_string(string, string, int, posn, posn, read_term).
> 
> Line is too long.

Fixed.

> Other than this, the changes to these files are ok.  I'll review the
> rest of them soon.

OK, thanks.

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



More information about the developers mailing list