From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Taylan Kammer Newsgroups: gmane.lisp.guile.bugs Subject: bug#72365: srfi-64: test-on-bad-end-name-simple is not allowed to raise an exception Date: Wed, 2 Oct 2024 23:07:01 +0200 Message-ID: <8320ebef-80f6-46ee-b01e-620282926f9c@gmail.com> References: <2a1fc413-ccd8-470a-9088-6d06386f32ec@gmail.com> <87ploi4rkw.fsf@wolfsden.cz> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="------------VAyOGZgLNiAafb3XHXOTKa8d" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="1982"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla Thunderbird Cc: 72365@debbugs.gnu.org To: Tomas Volf <~@wolfsden.cz> Original-X-From: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Wed Oct 02 23:09:20 2024 Return-path: Envelope-to: guile-bugs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1sw6b1-0000KI-Mo for guile-bugs@m.gmane-mx.org; Wed, 02 Oct 2024 23:09:20 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sw6am-0005hk-B4; Wed, 02 Oct 2024 17:09:04 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sw6aj-0005hS-QE for bug-guile@gnu.org; Wed, 02 Oct 2024 17:09:02 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1sw6aj-0006Vy-IF for bug-guile@gnu.org; Wed, 02 Oct 2024 17:09:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=In-Reply-To:From:References:MIME-Version:Date:To:Subject; bh=DUy/HpUS+MbSBVjyRHVJDx6OSO2vGfFEO933/nY4qwo=; b=hzmcRvLs2VLY/ph8DfOdoQ+RMu5iwhTgT2QLFH7q+rFEiY9qKgK1VKYX9OC6QzuM2+NZm5FagpLkAcUjx/o3gw/LZmH9zWIz+bV/F7xfN1L79dKkV9164tziNOWFFCUxVfDu1vIwTt5/dC+Q1lyq5USmaXeDiHzPxIj88l3ZCAczuxV3451GegZUJKBuISQeQHHaPwil+cHeNR3lbBUDfVGEbQTL2ro8TtsW7oHjYdz7qfHRXfU8mUn3cjs5SVd62IXZ31GP0MBelqrWGuqf+w0+S10sPMb5kgoAy0/HFDhYmv1xF1jrgwUIaEt/tSTqrP3kOOgWeQb8sAb6Z1Igxg==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1sw6ak-0001Go-5O for bug-guile@gnu.org; Wed, 02 Oct 2024 17:09:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Taylan Kammer Original-Sender: "Debbugs-submit" Resent-CC: bug-guile@gnu.org Resent-Date: Wed, 02 Oct 2024 21:09:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 72365 X-GNU-PR-Package: guile Original-Received: via spool by 72365-submit@debbugs.gnu.org id=B72365.17279032954811 (code B ref 72365); Wed, 02 Oct 2024 21:09:02 +0000 Original-Received: (at 72365) by debbugs.gnu.org; 2 Oct 2024 21:08:15 +0000 Original-Received: from localhost ([127.0.0.1]:59391 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sw6Zy-0001FW-Jc for submit@debbugs.gnu.org; Wed, 02 Oct 2024 17:08:15 -0400 Original-Received: from mail-wm1-f54.google.com ([209.85.128.54]:38476) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sw6Zv-0001FE-Ae for 72365@debbugs.gnu.org; Wed, 02 Oct 2024 17:08:13 -0400 Original-Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-42ca5447142so203995e9.2 for <72365@debbugs.gnu.org>; Wed, 02 Oct 2024 14:08:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727903224; x=1728508024; darn=debbugs.gnu.org; h=in-reply-to:from:content-language:references:cc:to:subject :user-agent:mime-version:date:message-id:from:to:cc:subject:date :message-id:reply-to; bh=DUy/HpUS+MbSBVjyRHVJDx6OSO2vGfFEO933/nY4qwo=; b=b+v6in+EetkHUuRn4g7SITLR8ryW3G3ARpwHVeP/BJzRADxik5/Z575jkw2718h3OV bh7CPImaGKmdKenE2gf6NGQs+QqQqExLpMaUUgt6yHCyNnhOU0HxOBAjyDWFtzEHRGcN RV7aghd0u5SwNQegsRlhz8WB8LZOc1oxt1PSqW9lOHv57m5S1wGHYUxJ/0/x/u1LyIp9 uSVV4OJt9Dit9CzYmsa6/B9gye5ufMCsTVQzfx3g4Ias+nRq4F/Yw25fVeaXFayvTy78 vHJKCbZMiz4sEsEAmRCrLSIXIVcRC3GdapcPwRVHsQzBTBRG5CLOXAikWtINqv37hp84 jkTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727903224; x=1728508024; h=in-reply-to:from:content-language:references:cc:to:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=DUy/HpUS+MbSBVjyRHVJDx6OSO2vGfFEO933/nY4qwo=; b=aEvLflDpzAv0/O30cd3XuPSseYTsI3BSu+kP5nheRmZOGFzO0fkrJclIzCUTTk8MRX oXINxa8f1+fl6Hs71BUd5mhKDXn5hqrnGALaYi9NzWRJZzHgrpkJNxgmm5iQWZzDl2ul L4wxtLanu7rzJ50cJ5iYsMLbPdzKHkmIh94020eHFSOB0W9wqxxpgRzfdDjiFBoQj00f rOJEnjfTG54wr4bpkjuxGz0jnv5nLqTqUeSqdxwYb+GCY6kkjkzoAR+IDRFle+zs35UG gQJ9yo0N9ji+VvgWgEF4nUPWekO8CKhBKDOWWO996gAfAzrdWQ4+bGd5dQOoHAcbdKnR PGBQ== X-Gm-Message-State: AOJu0Yxy1QqERTxUrQfEopXeFDGbEXud+cYA0w9iK8YHWvhAQi4/dXNY 4rnCMaBIXy54RjE2gQ9nsFG/j2bW+dY/WNumtVQn82DmcJMeztJF X-Google-Smtp-Source: AGHT+IHnn4dbIz+6ypfg0gRFsBfpjMYNbRfmXe749OjDOqE02jsriNL33SGa9PdEp3BfB9G6RxXklQ== X-Received: by 2002:a05:600c:a02:b0:42c:aeee:80a with SMTP id 5b1f17b1804b1-42f778f359fmr16402455e9.7.1727903223883; Wed, 02 Oct 2024 14:07:03 -0700 (PDT) Original-Received: from ?IPV6:2003:106:8f04:c300:23:dfca:ebd4:aa57? (p200301068f04c3000023dfcaebd4aa57.dip0.t-ipconnect.de. [2003:106:8f04:c300:23:dfca:ebd4:aa57]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-42f79ed3e3fsm28332885e9.23.2024.10.02.14.07.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 02 Oct 2024 14:07:03 -0700 (PDT) Content-Language: en-US In-Reply-To: <87ploi4rkw.fsf@wolfsden.cz> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-guile@gnu.org List-Id: "Bug reports for GUILE, GNU's Ubiquitous Extension Language" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Original-Sender: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.lisp.guile.bugs:11023 Archived-At: This is a multi-part message in MIME format. --------------VAyOGZgLNiAafb3XHXOTKa8d Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 02.10.2024 14:28, Tomas Volf wrote: > Taylan Kammer writes: > > Hi, > > sorry for taking so long to respond to your comments, work has been bit > busy lately. I took two months whereas you took two days, so I'm not going to complain. :D >> 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 Do I understand correctly that this is an additional test suite for testing SRFI-64 itself? Like the "meta test suite" shipped with SRFI-64? Is there a brief description somewhere on how to run it with Guile? Would be really neat if I can use it to further test my implementation. >> 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 Your implementation seems written specifically with Guile in mind, which is a big plus I guess. Mine was written with R7RS in mind, and has compatibility shims for Guile as well as other Scheme implementations like Kawa (shims adopted from the original implementation). This makes it hacky in some parts, like when it comes to how it gathers source code information for file name & line number reporting. It also lacks docstrings, doesn't make use of Guile features like `define*` and so on... If the quality of the implementations is the same or higher, in terms of observable behavior, then it should be preferred for Guile, I think. If I find the time, I'll see if I can use your implementation to run some of my test suites, like the bytestructures test suite, and report if I notice any issues. >> 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. That's true. I don't mean to insult the author or anything. It is what it is. >> 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. Makes sense I guess. I've generally tried to imitate the reference implementation's behavior as closely as possible in such matters, worrying that there might be code out there that relies on its various quirks, but maybe I'm being too paranoid. I don't have a strong opinion either way. The number of people, who want to write a test runner that does something special on bad-end-name (something other than raise an error), is probably very small. - Making `test-end` itself raise an error would probably be most convenient, so test runner authors don't have to take care of it. - But if `test-end` doesn't do it, it's not a big deal either IMO, because all they would need to do is to call `(test-runner-on-bad-end-name! my-runner test-on-bad-end-name-simple)` to make their custom runner raise an error as well. (And, if they want to do something before, they can use a procedure that ends with the call `(test-on-bad-end-name-simple ...)`.) The latter is my preference, because enabling the behavior via a single line of code is easy, whereas disabling it would be difficult / impossible if `test-end` were to be hardcoded to raise an error. But if a SRFI-64 implementation made its `test-end` always raise an error, it probably wouldn't anyone in practice, so I wouldn't see it as a real problem. > 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. Right, it doesn't make it useless per se. It could still be called before `test-end` raises an error. > Have a nice day, > Tomas Likewise! - Taylan --------------VAyOGZgLNiAafb3XHXOTKa8d Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
On 02.10.2024 14:28, Tomas Volf wrote:
Taylan Kammer <taylan.kammer@gmail.com> writes:

Hi,

sorry for taking so long to respond to your comments, work has been bit
busy lately.

I took two months whereas you took two days, so I'm not going to complain. :D


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

Do I understand correctly that this is an additional test suite for testing SRFI-64 itself? Like the "meta test suite" shipped with SRFI-64?

Is there a brief description somewhere on how to run it with Guile? Would be really neat if I can use it to further test my implementation.


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

Your implementation seems written specifically with Guile in mind, which is a big plus I guess.

Mine was written with R7RS in mind, and has compatibility shims for Guile as well as other Scheme implementations like Kawa (shims adopted from the original implementation). This makes it hacky in some parts, like when it comes to how it gathers source code information for file name & line number reporting. It also lacks docstrings, doesn't make use of Guile features like `define*` and so on...

If the quality of the implementations is the same or higher, in terms of observable behavior, then it should be preferred for Guile, I think. If I find the time, I'll see if I can use your implementation to run some of my test suites, like the bytestructures test suite, and report if I notice any issues.


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.

That's true. I don't mean to insult the author or anything. It is what it is. 

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.

Makes sense I guess. I've generally tried to imitate the reference implementation's behavior as closely as possible in such matters, worrying that there might be code out there that relies on its various quirks, but maybe I'm being too paranoid.

I don't have a strong opinion either way. The number of people, who want to write a test runner that does something special on bad-end-name (something other than raise an error), is probably very small.

- Making `test-end` itself raise an error would probably be most convenient, so test runner authors don't have to take care of it.

- But if `test-end` doesn't do it, it's not a big deal either IMO, because all they would need to do is to call `(test-runner-on-bad-end-name! my-runner test-on-bad-end-name-simple)` to make their custom runner raise an error as well. (And, if they want to do something before, they can use a procedure that ends with the call `(test-on-bad-end-name-simple ...)`.)

The latter is my preference, because enabling the behavior via a single line of code is easy, whereas disabling it would be difficult / impossible if `test-end` were to be hardcoded to raise an error. But if a SRFI-64 implementation made its `test-end` always raise an error, it probably wouldn't anyone in practice, so I wouldn't see it as a real problem.


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.

Right, it doesn't make it useless per se. It could still be called before `test-end` raises an error.


Have a nice day,
Tomas

Likewise!

- Taylan

--------------VAyOGZgLNiAafb3XHXOTKa8d--