Taylan Kammer <taylan.kam...@gmail.com> writes: Hi,
sorry for taking so long to respond to your comments, work has been bit busy lately. I really appreciate you looking at them and validating and/or challenging my conclusions. > On 30.07.2024 21:51, Tomas Volf wrote: >> Hello, >> >> I think I found a bug in (srfi srfi-64) module shipped with GNU Guile. > > Hi Tomas, > > Thanks for stress-testing the SRFI 64 spec & implementation and > reporting all these discrepancies. :-) No problem, it was necessary during implementing my own version of SRFI-64. The full test suite is available as part of my library[2], the runner[3] is quite simple, so using it to test your implementation should be fairly easy. I think there are few more bugs than I reported, I lost the willpower to do the rest somewhere on the way. :/ 2: https://git.wolfsden.cz/guile-wolfsden/tree/tests/srfi-64 3: https://git.wolfsden.cz/guile-wolfsden/tree/build-aux/srfi64test-driver.scm > > Firstly, to reiterate some things I've already mentioned in the thread on bug > 71300, just so it goes on record here as well: > > I have a SRFI 64 implementation of my own. I hope Guile will switch to it > eventually because I find the upstream reference implementation to be somewhat > unpleasant to work with. (It's monolithic, and not the cleanest code.) I plan to attempt to upstream my version as well, so I guess it is a race :) > > Until then, my implementation can be used by following these steps: > > 1. Cloning this repo: > > https://codeberg.org/taylan/scheme-srfis/ > > 2. Running Guile like so: > > GUILE_LOAD_PATH=/path/to/scheme-srfis/guile-srfi-64 guile > > (Replacing /path/to/scheme-srfis with the actual path to wherein the repo was > cloned, of course.) > > Then, loading SRFI-64 the regular way should load my implementation > rather than the one that ships with Guile (which is the reference > implementation from the SRFI author). You can find my version here[0]. If you do not use Guix, building from tarball[1] might be easier. Contrary to your version, mine is available as (wolfsden srfi srfi-64). 0: https://git.wolfsden.cz/guile-wolfsden/ 1: https://wolfsden.cz/project/guile-wolfsden.html > > I'll respond to your reports one by one, treating them like bug reports > towards > my own implementation, since it was originally derived from the reference > implementation and has probably inherited some of the bugs. You are braver than me. After few days of staring at the reference implementation (in Guile), I just said "nope" and wrote a new one from scratch. > Unfortunately, I'm not motivated to work on the implementation that's > in Guile, because I find it too cumbersome to navigate its code and > the unclean coding practices too distracting. While in principle I agree, let us not be too harsh, the implementation is really old. I assume coding practices available at the time to achieve portability were bit different. The implementation even considers SRFI-9 optional, these days I think that would be considered bit absurd. > >> The specification says the following about the simple test runner: >> >>> Creates a new simple test-runner, that prints errors and a summary on the >>> standard output port. >> It does not mention that it can signal errors, so I believe the following >> should >> just print to the terminal: >> >> (use-modules (srfi srfi-64)) >> (test-on-bad-end-name-simple (test-runner-null) "x" "y") >> >> However is signals an error instead: >> >> Backtrace: >> In ice-9/boot-9.scm: >> 1752:10 6 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _) >> In unknown file: >> 5 (apply-smob/0 #<thunk 7f6cf05a9300>) >> In ice-9/boot-9.scm: >> 724:2 4 (call-with-prompt ("prompt") #<procedure 7f6cf05b6280 at >> ice-9/eval.scm:330:…> …) >> In ice-9/eval.scm: >> 619:8 3 (_ #(#(#<directory (guile-user) 7f6cf05acc80>))) >> In ice-9/boot-9.scm: >> 2836:4 2 (save-module-excursion #<procedure 7f6cf059d300 at >> ice-9/boot-9.scm:4393:3 ()>) >> 4388:12 1 (_) >> In srfi/srfi-64/testing.scm: >> 375:14 0 (test-on-bad-end-name-simple _ _ _) >> >> srfi/srfi-64/testing.scm:375:14: In procedure >> test-on-bad-end-name-simple: >> test-end x does not match test-begin y > > The spec is very sparse on what the simple test runner does, so I'm > not sure if the intention is to imply that it does nothing other than > what's stated. I am not sure how to read the spec regarding this. But in my reading >>> Creates a new simple test-runner, that prints errors and a summary >>> on the standard output port. is clear enough. Same way I would think (for example) reporting a telemetry (e.g. on number of tests executed) would violate the spec. > > In one case, the reference implementation clearly violates the specification: > The simple test runner uses the `aux` field which the spec claims it doesn't > use. (My implementation fixes this.) However, in this case it's not that > clear-cut. > > In this case, I think raising an error is good default behavior, since the > mismatched end name indicates a problem with the test suite itself rather than > the code being tested. If it poses a problem to the user, one can override > that > callback with the `test-runner-on-bad-end-name!` setter. > > What do you think? I agree that raising an error is good behavior. However I do not think that on-bad-end-name-function is a place where to do it. In my opinion the name mismatch is a hard error, in my implementation subclass of &programming-error[4]. If I am writing new test runner, the specification does not mention that raising the error is *my* responsibility, just that test-end will signal an error. To rephrase that: test-end is mandated to signal error, but custom test runner has no provision requiring it to do it in on-bad-end-name-function. Hence I believe test-end needs to be the one to signal the error. However! That does not make on-bad-end-name-function useless. The specification does not mandate *how* the error signaled by test-end should look like, hence there is no *portable* way to detect it. Custom runner, if it needs to report name mismatch specially, can just produce specific log line in the callback (or even signal its own exception first before test-end does). 4: https://git.wolfsden.cz/guile-wolfsden/tree/wolfsden/srfi/srfi-64.scm#n960 Let me know what you think. Have a nice day, Tomas -- There are only two hard things in Computer Science: cache invalidation, naming things and off-by-one errors.
signature.asc
Description: PGP signature
