[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Two r6rs bugs
From: |
Mark H Weaver |
Subject: |
Re: Two r6rs bugs |
Date: |
Fri, 23 Nov 2012 01:29:17 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Hi Ian,
Ian Price <address@hidden> 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 <address@hidden>
> 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