Re: [PATCH] export/rename
On Tue, Oct 10, 2023 at 08:14:54AM +0200, Peter Bex wrote: > After some more thought on this, I think you convinced me. > Let's go with the original export/rename patch, unless anyone objects. A week has passed with no objections. I've pushed the original patch. Cheers, Peter signature.asc Description: PGP signature
Re: [PATCH] export/rename
On Mon, Oct 09, 2023 at 05:01:47PM +0200, felix.winkelm...@bevuta.com wrote: > The export/rename localizes the functionality and doesn't > require changes to existing code. The precedent of the syntax: and > interface: never was a particularly good one (we should have used separate > export forms for those right from the start), so adding funny markers or > possibly ambiguous special cases is not my favorite approach. After some more thought on this, I think you convinced me. Let's go with the original export/rename patch, unless anyone objects. Cheers, Peter signature.asc Description: PGP signature
Re: [PATCH] export/rename
> I'd prefer if we could develop this a bit further to make the rename > form accessible to module as well. Sure. I personally never felt the need for rename on export and wanted to add this just as a base to build R7RS support on. Bindings and aliases are so easily done in Scheme that it doesn't seem necessary to complicate existing forms further. > Not sure if that would be useful in functors. I've never used them, > but it seems to me that the functor's implementation is in a better > position to determine whether to rename on export or not, not the > functor definition itself. I agree. The functor is an _external_ interface, how the implementation names stuff inside is of no concern. > If you agree that it makes little sense to support it in functors, > perhaps it's better to call ##sys#syntax-error when the functor > definition includes renamed exports? Personally I prefer a separate form, but this makes sense of course. > I had a quick look and the main obstacle seems to be that > ##core#module is not very extendable, because it assumes the first two > "arguments" are special, and the rest is the body (IIUC). Correct, that#s why I began with a separate export form. > Would it perhaps be possible to simply expand (module FOO (exps) BODY) to > something like the following? > > (##core#module FOO (non-renamed-exps) > (export (rename: name1 renamed1) ...) > BODY) It's all the same. Let's keep ##core#module as it is (and perhaps "module" itself) I wonder whether we should expend so much thought about this, to be honest. The export/rename localizes the functionality and doesn't require changes to existing code. The precedent of the syntax: and interface: never was a particularly good one (we should have used separate export forms for those right from the start), so adding funny markers or possibly ambiguous special cases is not my favorite approach. cheers, felix
Re: [PATCH] export/rename
On Tue, Oct 03, 2023 at 12:51:57PM +0200, felix.winkelm...@bevuta.com wrote: > Ok, I find attached a variant, both more ugly in interface and > implementation, since ##core#module and functor do not yet allow > renamings to be handled. Also, "define-interface" has no notion > of this, so "(rename: OLD NEW)" is purely applicable in "export". I'd prefer if we could develop this a bit further to make the rename form accessible to module as well. Not sure if that would be useful in functors. I've never used them, but it seems to me that the functor's implementation is in a better position to determine whether to rename on export or not, not the functor definition itself. If you agree that it makes little sense to support it in functors, perhaps it's better to call ##sys#syntax-error when the functor definition includes renamed exports? > I prefer the first variant, but feel free to push which one you > like more. As it is, the first variant is cleaner indeed, but if we decide to support renames in the "module" form as well, I think we should go with the second variant. I had a quick look and the main obstacle seems to be that ##core#module is not very extendable, because it assumes the first two "arguments" are special, and the rest is the body (IIUC). Would it perhaps be possible to simply expand (module FOO (exps) BODY) to something like the following? (##core#module FOO (non-renamed-exps) (export (rename: name1 renamed1) ...) BODY) Perhaps a bit hand-wavey (sorry), but I think this is simpler than trying to change ##core#module. Cheers, Peter signature.asc Description: PGP signature
Re: [PATCH] export/rename
> Maybe this already works with the current patch, but can we support: > > (export (rename foo bar)) > > As well as the version with the colon (suffix keyword notation) on the end of > export? > Seems like that would be best for symmetry with the import form. > > (Sorry, it’s just aesthetics, I know.) Well, aesthetics is important. There are a couple of points, though: - import "rename" has a different syntax, as it combines an import spec with a rename list, here we effectively have a special case (3 argument list). - exports go through ##sys#validate-exports, so originally "export", "define-interface" and the export spec of the "module" form supported the same export notation, but now we have a special case for export, since we try to shoehorn renaming into the existing "export", but renaming is not allowed in "module" and "define-interface". The "(rename OLD NEW)" would be ambiguous with the old interpretation, of course. - having "rename" + "rename:" adds another special case where both keyword and normal symbol is allowed, I don't think we have this anywhere else. - My original intention was to provide a low-impact renaming export in the core in anticipation of re-using it for R7RS support. It still seems useful for CHICKEN 5, but since rename-on-export appears to me a seldom used feature I thought a separate form is sufficient. felix
Re: [PATCH] export/rename
Maybe this already works with the current patch, but can we support: (export (rename foo bar)) As well as the version with the colon (suffix keyword notation) on the end of export? Seems like that would be best for symmetry with the import form. (Sorry, it’s just aesthetics, I know.) Evan > On 3/10/2023, at 11:51 PM, felix.winkelm...@bevuta.com wrote: > >> On Mon, Oct 02, 2023 at 06:31:44PM +0200, felix.winkelm...@bevuta.com wrote: >>> This patch adds a new special form to explicitly export renamed bindings >>> from a module: >>> >>>(export/rename (OLD NEW) ...) >> >> Why not add it to the regular "export" form? It's already extendable, >> as it has syntax: and interface:. So for example we could have: >> >> (export (rename: OLD NEW) ...) >> >> Saves having yet another top-level special-case form. > > Ok, I find attached a variant, both more ugly in interface and > implementation, since ##core#module and functor do not yet allow > renamings to be handled. Also, "define-interface" has no notion > of this, so "(rename: OLD NEW)" is purely applicable in "export". > > I prefer the first variant, but feel free to push which one you > like more. > > > felix > <0001-allow-renaming-exports-in-export-form.patch>
Re: [PATCH] export/rename
> On Mon, Oct 02, 2023 at 06:31:44PM +0200, felix.winkelm...@bevuta.com wrote: > > This patch adds a new special form to explicitly export renamed bindings > > from a module: > > > > (export/rename (OLD NEW) ...) > > Why not add it to the regular "export" form? It's already extendable, > as it has syntax: and interface:. So for example we could have: > > (export (rename: OLD NEW) ...) > > Saves having yet another top-level special-case form. Ok, I find attached a variant, both more ugly in interface and implementation, since ##core#module and functor do not yet allow renamings to be handled. Also, "define-interface" has no notion of this, so "(rename: OLD NEW)" is purely applicable in "export". I prefer the first variant, but feel free to push which one you like more. felix From 93c8e56092b4cb462f0669ab9892213fa8090b71 Mon Sep 17 00:00:00 2001 From: felix Date: Tue, 3 Oct 2023 12:50:53 +0200 Subject: [PATCH] allow renaming exports in "export" form --- expand.scm | 44 +--- manual/Modules | 7 +++ modules.scm| 112 + tests/module-tests.scm | 22 4 files changed, 126 insertions(+), 59 deletions(-) diff --git a/expand.scm b/expand.scm index 13a7f553..a88ddda9 100644 --- a/expand.scm +++ b/expand.scm @@ -1175,7 +1175,10 @@ (cdr app)) ; functor arguments (else ;;XXX use module name in "loc" argument? -(let ((exports (##sys#validate-exports (strip-syntax (caddr x)) 'module))) +(let-values (((exports _) (##sys#validate-exports (strip-syntax (caddr x)) + 'module))) + ;;XXX we currently ignore renames here, to support this, we need to + ;; extend ##core#module `(##core#module ,name ,(if (eq? '* exports) @@ -1192,11 +1195,11 @@ 'export '() (##sys#er-transformer (lambda (x r c) -(let ((exps (##sys#validate-exports (strip-syntax (cdr x)) 'export)) - (mod (##sys#current-module))) - (when mod - (##sys#add-to-export-list mod exps)) - '(##core#undefined) +(let-values (((exps ren) (##sys#validate-exports (strip-syntax (cdr x)) 'export))) + (let ((mod (##sys#current-module))) +(when mod + (##sys#add-to-export-list mod exps ren)) +'(##core#undefined)) (##sys#extend-macro-environment 'reexport '() @@ -1223,17 +1226,20 @@ (##core#quote ,(library-id name)) (##core#quote ,(map (lambda (arg) - (let ((argname (car arg)) -(exps (##sys#validate-exports (cadr arg) 'functor))) -(unless (or (symbol? argname) -(and (list? argname) - (= 2 (length argname)) - (symbol? (car argname)) - (valid-library-specifier? (cadr argname - (##sys#syntax-error-hook "invalid functor argument" name arg)) -(cons argname exps))) + (let ((argname (car arg))) + (let-values (((exps _) (##sys#validate-exports (cadr arg) + 'functor))) + ;;XXX renames currently ignored + (unless (or (symbol? argname) + (and (list? argname) + (= 2 (length argname)) + (symbol? (car argname)) + (valid-library-specifier? (cadr argname +(##sys#syntax-error-hook "invalid functor argument" name arg)) + (cons argname exps args)) - (##core#quote ,(##sys#validate-exports exps 'functor)) + (##core#quote ,(let-values (((exps _) (##sys#validate-exports exps 'functor))) + exps)) (##core#quote ,body `(##core#module ,(library-id name) #t @@ -1260,7 +1266,11 @@ (cond ((eq? '* exps) '*) ((symbol? exps) `(#:interface ,exps)) ((list? exps) - (##sys#validate-exports exps 'define-interface)) + (let-values (((exps ren) (##sys#validate-exports exps 'define-interface))) + (unless (null? ren) +(syntax-error-hook 'define-interface + "renaming exports may not be used in interface definitions")) + exps)) (else (syntax-error-hook
Re: [PATCH] export/rename
On Mon, Oct 02, 2023 at 06:31:44PM +0200, felix.winkelm...@bevuta.com wrote: > This patch adds a new special form to explicitly export renamed bindings > from a module: > > (export/rename (OLD NEW) ...) Why not add it to the regular "export" form? It's already extendable, as it has syntax: and interface:. So for example we could have: (export (rename: OLD NEW) ...) Saves having yet another top-level special-case form. Cheers, Peter signature.asc Description: PGP signature
