chibi-scheme icon indicating copy to clipboard operation
chibi-scheme copied to clipboard

`(chibi parse)`: Improve failure continuation for parse-commit

Open nmeum opened this issue 4 years ago • 2 comments

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

https://github.com/ashinn/chibi-scheme/blob/5b8e196e0f9f6a65ff194aa6a64ffa88858e1be2/lib/chibi/parse/parse.scm#L524-L526

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 avatar Apr 09 '22 20:04 nmeum

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.

ashinn avatar Apr 11 '22 08:04 ashinn

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.

nmeum avatar Apr 11 '22 16:04 nmeum