unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
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



  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).