Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(chibi parse): Improve failure continuation for parse-commit #822

Open
nmeum opened this issue Apr 9, 2022 · 2 comments
Open

(chibi parse): Improve failure continuation for parse-commit #822

nmeum opened this issue Apr 9, 2022 · 2 comments

Comments

@nmeum
Copy link
Contributor

nmeum commented Apr 9, 2022

Currently, the parse-commit procedure from (chibi parse) unconditionally uses a failure continuation which simply returns #f, i.e. (lambda (s i r) #f):

(define (parse-commit f)
(lambda (source index sk fk)
(f source index (lambda (res s i fk) (sk res s i (lambda (s i r) #f))) fk)))

I encountered this while working on a parser which uses (chibi parse) with a custom error handling procedure / failure continuation as passed initially to call-with-parse. Since parse-commit does (correctly) not use the fk procedure argument it ends up not invoking this "top-level" failure continuation and simply causes call-with-parse to return #f which I found somewhat surprising and caused a bug in my code.

I was wondering if there was any interest in enhancing the parse-commit behavior in this regard. For example, by storing the initial "top-level" failure continuation (passed to call-with-parse) in the Parse-Stream record type and invoking that from parse-commit directly (thus still preventing backtracking) or by (optionally) allowing a custom failure continuation to be passed to parse-commit, e.g.:

-(define (parse-commit f)
-  (lambda (source index sk fk)
-    (f source index (lambda (res s i fk) (sk res s i (parse-stream-fk source))) fk)))
+(define (parse-commit f . o)
+  (let ((commit-fk (if (pair? o) (car o) (lambda (s i r) #f))))
+    (lambda (source index sk fk)
+      (f source index (lambda (res s i fk) (sk res s i commit-fk)) fk))))

I would personally find the former approach more intuitive but maybe that's just me. Thoughts?

nmeum added a commit to nmeum/edward that referenced this issue Apr 10, 2022
Otherwise, this causes repl-parse to be called recursively since the
parser is only advanced to the next line in the repl-parse failure
continuation.

See ashinn/chibi-scheme#822
@ashinn
Copy link
Owner

ashinn commented Apr 11, 2022

I'm fine with passing a custom failure continuation to parse-commit. This is more general in that it allows you to commit only up to a certain point if desired.

There are different ways this could be managed more conveniently which we could consider later. I'm not so sure about storing this in the Parse-Stream.

@nmeum
Copy link
Contributor Author

nmeum commented Apr 11, 2022

I created #824 to allow passing a custom failure continuation as a second argument and adjusted the documentation accordingly (also emphazising that prior failure continuations are not picked up by default).

There are different ways this could be managed more conveniently which we could consider later. I'm not so sure about storing this in the Parse-Stream.

Yep, I agree that storing stuff like that in the Parse-Stream is not ideal. I think this problem can be addressed separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants