bug#67255: define-library does not support 'rename' directives

bug#67255: define-library does not support 'rename' directives

[Maxim Cournoyer]

Hi,

Our R7RS define-library syntax, from (ice-9 r7rs-library) does not
support renaming bindings to export, via 'rename' directives.  For
example, attempting to build srfi/125.sld, which reads:

--8<---------------cut here---------------start------------->8---
(define-library (srfi srfi-125)

  (export

   make-hash-table
   hash-table
   hash-table-unfold
   alist->hash-table

   hash-table?
   hash-table-contains?
   hash-table-empty?
   hash-table=?
   hash-table-mutable?

   hash-table-ref
   hash-table-ref/default

   hash-table-set!
   hash-table-delete!
   hash-table-intern!
   hash-table-update!
   hash-table-update!/default
   hash-table-pop!
   hash-table-clear!

   hash-table-size
   hash-table-keys
   hash-table-values
   hash-table-entries
   hash-table-find
   hash-table-count

   hash-table-map
   hash-table-for-each
   hash-table-map!
   hash-table-map->list
   hash-table-fold
   hash-table-prune!

   hash-table-copy
   hash-table-empty-copy
   hash-table->alist

   hash-table-union!
   hash-table-intersection!
   hash-table-difference!
   hash-table-xor!

   ;; The following procedures are deprecated by SRFI 125:

   (rename deprecated:hash                     hash)
   (rename deprecated:string-hash              string-hash)
   (rename deprecated:string-ci-hash           string-ci-hash)
   (rename deprecated:hash-by-identity         hash-by-identity)

   (rename deprecated:hash-table-equivalence-function
                                               hash-table-equivalence-function)
   (rename deprecated:hash-table-hash-function hash-table-hash-function)
   (rename deprecated:hash-table-exists?       hash-table-exists?)
   (rename deprecated:hash-table-walk          hash-table-walk)
   (rename deprecated:hash-table-merge!        hash-table-merge!)

   )

  (import (scheme base)
          (scheme write) ; for warnings about deprecated features
          (srfi 126)
          (except (srfi 128)
                  hash-salt      ; exported by (srfi 126)
                  string-hash    ; exported by (srfi 126)
                  string-ci-hash ; exported by (srfi 126)
                  symbol-hash    ; exported by (srfi 126)
                  ))

  (cond-expand
   ((library (scheme char))
    (import (scheme char)))
   (else
    (begin (define string-ci=? string=?))))

  (include "srfi-125/125.body.scm")

  )
--8<---------------cut here---------------end--------------->8---

Fails with:

--8<---------------cut here---------------start------------->8---
$ ./meta/guild compile -W3 ./module/srfi/srfi-125.scm
/module/srfi/srfi-128.scm.go
ice-9/boot-9.scm:1682:22: In procedure raise-exception:
Syntax error:
unknown location: source expression failed to match any pattern in form ((rename deprecated:hash hash) (rename deprecated:string-hash string-hash) (rename deprecated:string-ci-hash string-ci-hash) (rename deprecated:hash-by-identity hash-by-identity) (rename deprecated:hash-table-equivalence-function hash-table-equivalence-function) (rename deprecated:hash-table-hash-function hash-table-hash-function) (rename deprecated:hash-table-exists? hash-table-exists?) (rename deprecated:hash-table-walk hash-table-walk) (rename deprecated:hash-table-merge! hash-table-merge!))
--8<---------------cut here---------------end--------------->8---

Our define-module syntax does not have such a feature (of renaming
*exported* bindings), so this would seem to require new development on
that side first.

-- 
Thanks,
Maxim

bug#67255: define-library does not support 'rename' directives

[Timothy Sample]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Our R7RS define-library syntax, from (ice-9 r7rs-library) does not
> support renaming bindings to export, via 'rename' directives.

I appreciate your R7RS debugging effort.  Thanks!

> Our define-module syntax does not have such a feature (of renaming
> *exported* bindings), so this would seem to require new development on
> that side first.

I believe you’re mistaken.  At least, the manual says:

 -- syntax: export variable ...
     Add all VARIABLEs (which must be symbols or pairs of symbols) to
     the list of exported bindings of the current module.  If VARIABLE
     is a pair, its ‘car’ gives the name of the variable as seen by the
     current module and its ‘cdr’ specifies a name for the binding in
     the current module’s public interface.

Using pairs in Guile’s ‘export’ (or ‘#:export’ in ‘define-module’)
should be the same as ‘rename’ from R7RS.

HTH!


-- Tim

bug#67255: define-library does not support 'rename' directives

[Timothy Sample]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

Timothy Sample <samplet@ngyro.com> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> Our R7RS define-library syntax, from (ice-9 r7rs-library) does not
>> support renaming bindings to export, via 'rename' directives.
>
> I appreciate your R7RS debugging effort.  Thanks!

Actions speak louder than words, so here’s a patch!

The ‘define-library’ syntax uses the R6RS ‘library’ syntax under the
hood.  TIL that R6RS and R7RS have different syntax for 'rename'.  In
R6RS, you write:

    (export (rename (internal external)))

while in R7RS, it’s:

    (export (rename internal external))

My patch adds a conversion step to deal with this difference.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-R7RS-rename-syntax-for-exports.patch --]
[-- Type: text/x-patch, Size: 1741 bytes --]

From b87bf8910ac8e75dc0ec63cb7385ddf199fd400c Mon Sep 17 00:00:00 2001
From: Timothy Sample <samplet@ngyro.com>
Date: Mon, 20 Nov 2023 11:01:08 -0600
Subject: [PATCH] Use R7RS 'rename' syntax for exports.

Fixes <https://bugs.gnu.org/67255>.
Reported by Maxim Cournoyer <maxim.cournoyer@gmail.com>.

* module/ice-9/r7rs-libraries.scm (define-library): Convert R7RS
exports to R6RS exports before passing them on to 'library'.
---
 module/ice-9/r7rs-libraries.scm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/module/ice-9/r7rs-libraries.scm b/module/ice-9/r7rs-libraries.scm
index 63a300a26..f8b6b4c59 100644
--- a/module/ice-9/r7rs-libraries.scm
+++ b/module/ice-9/r7rs-libraries.scm
@@ -1,5 +1,5 @@
 ;; R7RS library support
-;;      Copyright (C) 2020, 2021 Free Software Foundation, Inc.
+;;      Copyright (C) 2020, 2021, 2023 Free Software Foundation, Inc.
 ;;
 ;; This library is free software; you can redistribute it and/or
 ;; modify it under the terms of the GNU Lesser General Public
@@ -97,12 +97,17 @@
            ((decl ...)
             (partition-decls #'(decl ... . decls) exports imports code))))))
 
+    (define (r7rs-export->r6rs-export export)
+      (syntax-case export (rename)
+        ((rename internal external) #'(rename (internal external)))
+        (_ export)))
+
     (syntax-case stx ()
       ((_ name decl ...)
        (call-with-values (lambda ()
                            (partition-decls #'(decl ...) '() '() '()))
          (lambda (exports imports code)
            #`(library name
-               (export . #,exports)
+               (export . #,(map r7rs-export->r6rs-export exports))
                (import . #,imports)
                . #,code)))))))
-- 
2.41.0

bug#67255: define-library does not support 'rename' directives

[Maxim Cournoyer]

Hello!

Timothy Sample <samplet@ngyro.com> writes:

> Timothy Sample <samplet@ngyro.com> writes:
>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>>
>>> Our R7RS define-library syntax, from (ice-9 r7rs-library) does not
>>> support renaming bindings to export, via 'rename' directives.
>>
>> I appreciate your R7RS debugging effort.  Thanks!
>
> Actions speak louder than words, so here’s a patch!
>
> The ‘define-library’ syntax uses the R6RS ‘library’ syntax under the
> hood.  TIL that R6RS and R7RS have different syntax for 'rename'.  In
> R6RS, you write:
>
>     (export (rename (internal external)))
>
> while in R7RS, it’s:
>
>     (export (rename internal external))
>
> My patch adds a conversion step to deal with this difference.

Oh, excellent, thank you!

>>From b87bf8910ac8e75dc0ec63cb7385ddf199fd400c Mon Sep 17 00:00:00 2001
> From: Timothy Sample <samplet@ngyro.com>
> Date: Mon, 20 Nov 2023 11:01:08 -0600
> Subject: [PATCH] Use R7RS 'rename' syntax for exports.
>
> Fixes <https://bugs.gnu.org/67255>.
> Reported by Maxim Cournoyer <maxim.cournoyer@gmail.com>.

Nitpick: at least 'Reported-by' is a common git trailer, and these
must appear at the bottom of the git commit.

> * module/ice-9/r7rs-libraries.scm (define-library): Convert R7RS
> exports to R6RS exports before passing them on to 'library'.
> ---
>  module/ice-9/r7rs-libraries.scm | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/module/ice-9/r7rs-libraries.scm b/module/ice-9/r7rs-libraries.scm
> index 63a300a26..f8b6b4c59 100644
> --- a/module/ice-9/r7rs-libraries.scm
> +++ b/module/ice-9/r7rs-libraries.scm
> @@ -1,5 +1,5 @@
>  ;; R7RS library support
> -;;      Copyright (C) 2020, 2021 Free Software Foundation, Inc.
> +;;      Copyright (C) 2020, 2021, 2023 Free Software Foundation, Inc.
>  ;;
>  ;; This library is free software; you can redistribute it and/or
>  ;; modify it under the terms of the GNU Lesser General Public
> @@ -97,12 +97,17 @@
>             ((decl ...)
>              (partition-decls #'(decl ... . decls) exports imports code))))))
>  
> +    (define (r7rs-export->r6rs-export export)
> +      (syntax-case export (rename)
> +        ((rename internal external) #'(rename (internal external)))
> +        (_ export)))
> +
>      (syntax-case stx ()
>        ((_ name decl ...)
>         (call-with-values (lambda ()
>                             (partition-decls #'(decl ...) '() '() '()))
>           (lambda (exports imports code)
>             #`(library name
> -               (export . #,exports)
> +               (export . #,(map r7rs-export->r6rs-export exports))
>                 (import . #,imports)
>                 . #,code)))))))

It at least works for my use case (SRFI 128), so it's for sure an
improvement :-).  You can see it in action in the series I've sent today.

-- 
Thanks,
Maxim

bug#67255: define-library does not support 'rename' directives

[Timothy Sample]

Hey,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Timothy Sample <samplet@ngyro.com> writes:
>
>> Fixes <https://bugs.gnu.org/67255>.
>> Reported by Maxim Cournoyer <maxim.cournoyer@gmail.com>.
>
> Nitpick: at least 'Reported-by' is a common git trailer, and these
> must appear at the bottom of the git commit.

That’s a fair point.  I’m following what seems to be (from the commit
log) Guile’s convention here.  See

    $ git log --grep='^Report'

Whether it’s a good convention is probably out of scope here!  :)

> It at least works for my use case (SRFI 128), so it's for sure an
> improvement :-).  You can see it in action in the series I've sent today.

Hooray!


-- Tim

bug#67255: [PATCH v2] Use R7RS 'rename' syntax for exports.

[Maxim Cournoyer]

From: Timothy Sample <samplet@ngyro.com>

* module/ice-9/r7rs-libraries.scm (define-library): Convert R7RS
exports to R6RS exports before passing them on to 'library'.

Fixes: https://bugs.gnu.org/67255
Reported-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>.
Modified-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---

Changes in v2:
 - Improve pattern variable names

 module/ice-9/r7rs-libraries.scm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/module/ice-9/r7rs-libraries.scm b/module/ice-9/r7rs-libraries.scm
index 63a300a26..429d82ad9 100644
--- a/module/ice-9/r7rs-libraries.scm
+++ b/module/ice-9/r7rs-libraries.scm
@@ -1,5 +1,5 @@
 ;; R7RS library support
-;;      Copyright (C) 2020, 2021 Free Software Foundation, Inc.
+;;      Copyright (C) 2020, 2021, 2023 Free Software Foundation, Inc.
 ;;
 ;; This library is free software; you can redistribute it and/or
 ;; modify it under the terms of the GNU Lesser General Public
@@ -97,12 +97,18 @@
            ((decl ...)
             (partition-decls #'(decl ... . decls) exports imports code))))))
 
+    (define (r7rs-export->r6rs-export export-spec)
+      (syntax-case export-spec (rename)
+        ((rename from-identifier to-identifier)
+         #'(rename (from-identifier to-identifier)))
+        (identifier #'identifier)))
+
     (syntax-case stx ()
       ((_ name decl ...)
        (call-with-values (lambda ()
                            (partition-decls #'(decl ...) '() '() '()))
          (lambda (exports imports code)
            #`(library name
-               (export . #,exports)
+               (export . #,(map r7rs-export->r6rs-export exports))
                (import . #,imports)
                . #,code)))))))

base-commit: d579848cb5d65440af5afd9c8968628665554c22
-- 
2.41.0