From: Mark H Weaver <mhw@netris.org>
To: Ian Price <ianprice90@googlemail.com>
Cc: guile-devel@gnu.org
Subject: Re: Two r6rs bugs
Date: Fri, 23 Nov 2012 01:29:17 -0500 [thread overview]
Message-ID: <877gpc4yte.fsf@tines.lan> (raw)
In-Reply-To: <873902x6v9.fsf@googlemail.com> (Ian Price's message of "Thu, 22 Nov 2012 10:35:38 +0000")
Hi Ian,
Ian Price <ianprice90@googlemail.com> writes:
> The second one is a change to resolve-r6rs-interface. Previously
> mark-weaver [0], changes this so that it would correctly look up
> submodules under the srfi namespace, but in doing so took into account
> the srfi 97[1] library name, which it should not have done. I have added a
> comment to this effect in the source.
Indeed, good catch! However, I'd prefer to fix this differently.
See below.
> From 3c73a30c89e005927dcd6239b54e752c05c2a48f Mon Sep 17 00:00:00 2001
> From: Ian Price <ianprice90@googlemail.com>
> Date: Thu, 22 Nov 2012 10:16:44 +0000
> Subject: [PATCH 2/2] R6RS srfi library names should ignore first identifier
> after the :n
>
> * module/ice-9/r6rs-libraries.scm (resolve-r6rs-interface):
> (srfi :n name ids ...) -> (srfi srfi-n ids ...)
> * test-suite/tests/rnrs-libraries.test ("srfi"): Add test.
> ---
> module/ice-9/r6rs-libraries.scm | 6 +++++-
> test-suite/tests/rnrs-libraries.test | 4 +++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/module/ice-9/r6rs-libraries.scm b/module/ice-9/r6rs-libraries.scm
> index 019a6a7..9fef7a2 100644
> --- a/module/ice-9/r6rs-libraries.scm
> +++ b/module/ice-9/r6rs-libraries.scm
> @@ -40,7 +40,11 @@
> (substring (symbol->string (syntax->datum #'colon-n))
> 1)))))
> (resolve-r6rs-interface
> - #`(library (srfi #,srfi-n rest ... (version ...))))))
> + (if (null? #'(rest ...))
> + #`(library (srfi #,srfi-n (version ...)))
> + ;; SRFI 97 says that the first identifier after the colon-n
> + ;; is used for the libraries name, so it must be ignored.
> + #`(library (srfi #,srfi-n #,@(cdr #'(rest ...)) (version ...)))))))
Instead of using 'null?' and 'cdr' on the syntax object, can you please
rework this to use 'syntax-case'? I.e. instead of (if (null? ...) ...)
do this:
(syntax-case #'(rest ...) ()
(() <null-case>)
((name rest ...) <non-null-case>))
What do you think?
Other than that, it looks good to me.
Thanks,
Mark
next prev parent reply other threads:[~2012-11-23 6:29 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-22 10:35 Two r6rs bugs Ian Price
2012-11-22 11:38 ` Mike Gran
2012-11-22 12:05 ` Ian Price
2012-11-22 12:18 ` Mike Gran
2012-11-22 21:28 ` Ludovic Courtès
2012-11-23 6:29 ` Mark H Weaver [this message]
2012-11-25 12:30 ` Ian Price
2012-11-25 15:58 ` Mark H Weaver
2012-11-25 16:28 ` Ian Price
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877gpc4yte.fsf@tines.lan \
--to=mhw@netris.org \
--cc=guile-devel@gnu.org \
--cc=ianprice90@googlemail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).