[m-rev.] Re: for review: parsing_utils improvements

Ian MacLarty maclarty at csse.unimelb.edu.au
Tue Sep 29 15:21:19 AEST 2009


On Tue, Sep 29, 2009 at 2:30 PM, Ralph Becket <rafe at csse.unimelb.edu.au> wrote:
> Hi Ian,
>
> Paul forwarded your diff to me (I have an over eager spam filter...).  I
> have some Real Objections!
>
> (1) You call record_progress all over the place.  Why not just in, say,
> the next_char predicate?  The code is pretty ugly with all those
> impure scopes.
>

Because, as I understand it, not all the predicates in the interface
go through next_char.

> (2) Every call to record_progress makes an allocation and updates
> your error reference.  This is a Bad Thing for performance.
>

Yes, but from my point of view I'm more interesting in writing parsers
quickly than making them go fast.

What if I add a second mutable that was updated by record_progress
that only contained the current offset.  This would avoid an
allocation and a dereference everytime record_progress was called.
Then in (the relatively rarely called) fail_with_message I could
record the message and offset in an error_info mutable as I currently
do.  It would also update the offset in the second mutable.  Then when
it comes time to decide what offset to return in the parse predicate I
can compare the offset in the error_info and the new mutable to see
what to return.

> (3) Why are errors not part of the ordinary parser result?

I tried that, but it seems like a painful way to write a parser.
Perhaps I wasn't using parsing_utils the way it was intended?

> If you want
> a "quick bail out" error scheme, you can throw an exception.
>
> (4) Making parse cc_multi is unfortunate.
>

I agree.

Ian.

--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------



More information about the reviews mailing list