[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