New reference implementation taylanbayirli@xxxxxx (29 Aug 2015 17:38 UTC)
Re: New reference implementation Per Bothner (29 Aug 2015 20:23 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (29 Aug 2015 21:38 UTC)
Re: New reference implementation Per Bothner (30 Aug 2015 09:20 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (30 Aug 2015 10:45 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (31 Aug 2015 21:22 UTC)
Re: New reference implementation Per Bothner (31 Aug 2015 22:11 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (01 Sep 2015 08:44 UTC)
Re: New reference implementation Per Bothner (01 Sep 2015 10:44 UTC)
Re: New reference implementation John Cowan (30 Aug 2015 01:24 UTC)
Re: New reference implementation Arthur A. Gleckler (30 Aug 2015 04:35 UTC)
Re: New reference implementation John Cowan (30 Aug 2015 17:10 UTC)
Re: New reference implementation Per Bothner (30 Aug 2015 05:06 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (30 Aug 2015 08:06 UTC)
Re: New reference implementation Per Bothner (30 Aug 2015 08:25 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (30 Aug 2015 08:49 UTC)
Re: New reference implementation Per Bothner (30 Aug 2015 09:33 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (30 Aug 2015 12:35 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (22 Sep 2015 21:27 UTC)
Re: New reference implementation Per Bothner (24 Sep 2015 00:25 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (24 Sep 2015 08:26 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (26 Sep 2015 11:49 UTC)
Re: New reference implementation Per Bothner (28 Sep 2015 17:47 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (28 Sep 2015 19:54 UTC)
Re: New reference implementation Per Bothner (02 Oct 2015 06:07 UTC)
Re: New reference implementation Per Bothner (02 Oct 2015 06:36 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (02 Oct 2015 09:39 UTC)
Re: New reference implementation Per Bothner (06 Oct 2015 21:13 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (07 Oct 2015 09:17 UTC)

New reference implementation taylanbayirli@xxxxxx 29 Aug 2015 17:38 UTC

With all due respect to the author(s), the SRFI-64 reference
implementation is somewhat problematic.

- The style of the code is quite non-idiomatic Scheme.
- Sometimes over-complicated.
- Internally inconsistent.
- Sometimes the indentation is wrong, in part seriously inhibiting
  readability.
- It *does* have concrete bugs that seem to stem from these issues.
- It doesn't conform to its own specification!
- Gratuitous use of cond-expand doesn't help either.

One might say that the best way to move forward is identify the actual
bugs in it and solve them, not touching the rest so as not to introduce
any new possible bugs.

But for the time being, I have little motivation to go that route.  It's
mentally straining for me to go through this type of code.  I wish to
allocate my time and energy to other things.

That aside, I forgot what all the bugs I encountered were (shame on me,
I didn't note them down).  At the bottom there's a list of the ones I
could dig out from IRC logs and memory, and peeking into the code to
some degree after all.

---

However, I wrote a clean almost-from-scratch implementation in R7RS a
while back as part of my "SRFI implementations in R7RS" project:

https://github.com/TaylanUB/scheme-srfis/blob/master/srfi/64.body.scm

- The size of the code is halved (1k to 500).
- The code is simplified, idiomatic, and consistent.
- There's no cond-expand; it's pure R7RS.
- It follows the spec carefully.
- It doesn't attempt to record line numbers itself, since test
  expressions are named anyhow (the expression itself is printed when
  the name is omitted), so I think it's an insignificant feature.
- The output it prints by default is arguably nicer.
- It doesn't litter your working directory with a log file you didn't
  ask for and which contains no more than the standard output.

And you can still make it do whatever you want by providing your own
test-runner object, as written in the spec.

It's only been tested moderately by me but I'm going to go on a limb and
say that I'm more confident in its overall bug-free-ness than the old
implementation, because clean code goes a long way.  It will be easier
to debug too.

I would be very honored if I could get the approval of the SRFI author,
Per Bothner (CC), to sanction this as the primary reference
implementation.  I would then work on a Guile port (my favored Scheme
implementation), and maybe some other platforms that don't support R7RS
yet, especially if they adopted the old reference implementation into
their source tree like Guile has.

What do you think?

---

The promised list of issues for if you say no, and just want to fix them
in the existing implementation:

- The specification says nothing about log files.  Maybe it should at
  least be disabled by default.

- The 'test-runner-on-test-end' procedure is called, on line 569, by
  '%test-report-result'.  Why not '%test-on-test-end'?  I'm actually not
  sure if this leads to an actual bug, but it's very confusing while
  reading the code.

- Line 645: If a test will be skipped, on-test-begin is still called.
  This is most certainly a bug as per the specification.

- On line 758 which defines 'test-assert', the 'tname' argument to the
  macro is evaluated by being bound to the variable 'name', but later
  'tname' is used again and not 'name', and that in a quoted list, so
  this 'test-assert' only takes literals for the test name.  (This
  breaches the specification which says it is evaluated.)  This bug is
  partly hidden by the fact that it's in the else branch of a
  cond-expand.

- On line 704, the other definition of 'test-assert' (for Kawa,
  MzScheme, and Guile 2), the same mistake of binding 'name' to the
  evaluation of 'tname', but then using 'tname' instead of 'name'
  occurs.  This time 'tname' is *not* in a quoted list so in effect it
  will be evaluated twice (breaching the spec), though this is a minor
  bug.  It's noteworthy that there's no particular reason it's quoted in
  one definition and not the other (the other just uses 'cons'); it's
  simply the code being inconsistent in its style.

In my opinion, most of these bugs could have been avoided if the style
of the code were more disciplined and simple such that it puts less
cognitive load on the programmer.  I don't mean to be some code
cleanness pedant with no touch with reality; I'm doing my best to be
still pragmatic. :-)

This might be all, or there might be one or two that I don't remember.
Sorry for being somewhat lousy there.

Kind regards,
Taylan