[PATCH] SRFI-34, SRFI-60 and core bindings

[PATCH] SRFI-34, SRFI-60 and core bindings

[Ludovic Courtès]

Hi,

The patch below fixes SRFI-34 and SRFI-60 so that they don't override
core bindings.  The nice side-effect is that it suppresses annoying
warning messages.

Thanks,
Ludovic.


2005-10-20  Ludovic Courtès  <ludovic.courtes@laas.fr>

	* srfi/srfi-34.scm: Don't export `raise', replace it instead
	  (because `raise' otherwise overrides a code binding).
    
	* test-suite/tests/srfi-34.test (define-module): Don't pass a
	  `:duplicate' argument.
    
	* srfi/srfi-60.scm: Don't export `bit-count', replace it
          instead.


--- orig/srfi/srfi-34.scm
+++ mod/srfi/srfi-34.scm
@@ -27,8 +27,8 @@
 ;;; Code:
 
 (define-module (srfi srfi-34)
-  #:export (with-exception-handler
-	    raise)
+  #:export (with-exception-handler)
+  #:replace (raise)
   #:export-syntax (guard))
 
 (cond-expand-provide (current-module) '(srfi-34))


--- orig/srfi/srfi-60.scm
+++ mod/srfi/srfi-60.scm
@@ -22,7 +22,6 @@
 	    bitwise-xor
 	    bitwise-not
 	    any-bits-set?
-	    bit-count
 	    bitwise-if bitwise-merge
 	    log2-binary-factors first-set-bit
 	    bit-set?
@@ -42,7 +41,8 @@
 	       logtest
 	       logcount
 	       logbit?
-	       ash))
+	       ash)
+  #:replace (bit-count))
 
 (load-extension "libguile-srfi-srfi-60-v-1" "scm_init_srfi_60")
 

--- orig/test-suite/tests/srfi-34.test
+++ mod/test-suite/tests/srfi-34.test
@@ -18,7 +18,6 @@
 ;;;; Boston, MA 02110-1301 USA
 
 (define-module (test-suite test-srfi-34)
-  :duplicates (last)  ;; avoid warning about srfi-34 replacing `raise'
   :use-module (test-suite lib)
   :use-module (srfi srfi-13)
   :use-module (srfi srfi-34))




_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] SRFI-34, SRFI-60 and core bindings

[Kevin Ryde]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
> +  #:replace (bit-count))

I don't really want to silently replace the core bit-count, the
srfi-60 one is completely different.  It's pretty annoying to get a
warning or have to use #:renamer, but I don't know a better way.

(Incidentally, `current-time' from srfi-19 is also a problem.)


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] SRFI-34, SRFI-60 and core bindings

[Ludovic Courtès]

Kevin Ryde <user42@zip.com.au> writes:

> I don't really want to silently replace the core bit-count, the
> srfi-60 one is completely different.  It's pretty annoying to get a
> warning or have to use #:renamer, but I don't know a better way.

`#:replace' _is_ this better way: it does _not_ override the core
binding, unlike `(use-modules (srfi srfi-60))' (with no renamer) in the
current state.  What it does is that is replaces this binding only
within the module user: the binding replacement is confined.

  guile> (define-module (chbouib))
  #<directory (chbouib) 30083e90>
  guile> (use-modules (srfi srfi-60))
  guile> (bit-count 2)
  1
  guile> (set-current-module (resolve-module '(guile-user)))
  #<directory (chbouib) 30083e90>
  guile> (bit-count #f (make-uniform-vector 8 #t #f))
  8

This is exactly the behavior users may expect.

> (Incidentally, `current-time' from srfi-19 is also a problem.)

Right, it also annoyed me in the past.  If we agree that `#:replace' is
the right thing, then we can do the same for this one.

Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] SRFI-34, SRFI-60 and core bindings

[Kevin Ryde]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
> it does _not_ override the core
> binding, unlike `(use-modules (srfi srfi-60))' (with no renamer) in the
> current state.  What it does is that is replaces this binding only
> within the module user: the binding replacement is confined.

Yes, I'm talking about the module user too, the module user loses some
core bindings.

> This is exactly the behavior users may expect.

If you don't know about the clashes/replacements then you're likely to
be unpleasantly surprised to see core stuff suddenly move under your
feet.  But a way to acknowledge that in the use-modules might be nice.

An option for srfi-60 bit-count would be to extend the core, since the
argument types are different in the two forms.  Such cases have been
using #:replace, eg. srfi-17 and srfi-39.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] SRFI-34, SRFI-60 and core bindings

[Ludovic Courtès]

Hi,

Kevin Ryde <user42@zip.com.au> writes:

> Yes, I'm talking about the module user too, the module user loses some
> core bindings.
>
>> This is exactly the behavior users may expect.
>
> If you don't know about the clashes/replacements then you're likely to
> be unpleasantly surprised to see core stuff suddenly move under your
> feet.  But a way to acknowledge that in the use-modules might be nice.

My opinion about this is that it is a matter of documentation.  That is,
you don't want core bindings to get overloaded in unexpected ways.

However, in some cases, you do know that a given module redefines
various core bindings, and you do know that you *want* this.  The manual
clearly documents the binding conflicts for the SRFI modules we're
talking about.  I'd say it is the user's responsibility to make sure
they know what they're doing.  ;-)

As I tried to explain in the doc, `#:replace' is really a way for the
module developer to give a *hint* to the module user.  By default, this
hint will be obeyed by the module user.  However, the user still has the
opportunity to not take this hint into account by choosing a different
chain of duplicate binding handlers.

Now, if that hint is not provided by the modules in question and you
want to tell the module system that you *know* what you're doing (to get
rid of the warning message), then there is no clean way to do it.  Of
course, you can choose the `last', `first', or whatever duplicate
binding handler that makes the warning message vanish (this is what
people sometimes do currently).  But that does not qualify as a clean
solution.

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] SRFI-34, SRFI-60 and core bindings

[Marius Vollmer]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> 	* srfi/srfi-34.scm: Don't export `raise', replace it instead
> 	  (because `raise' otherwise overrides a code binding).

These two 'raise' funtions are very different: one raises a Unix
signal, the other raises an exception.  So I'd say the warning is OK.

> 	* srfi/srfi-60.scm: Don't export `bit-count', replace it
>           instead.

The same is true for 'bit-count' as Kevin pointed out. 

If you don't want to warning, you can define your own way of handling
duplicates.  See the NEWS file for docs about the options.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] SRFI-34, SRFI-60 and core bindings

[Ludovic Courtès]

Marius Vollmer <mvo@zagadka.de> writes:

> These two 'raise' funtions are very different: one raises a Unix
> signal, the other raises an exception.  So I'd say the warning is OK.

[...]

> The same is true for 'bit-count' as Kevin pointed out. 

I agree that these are way different functions, but I disagree about the
remedy.  You say that a warning is fine precisely because these are
different functions.  I say that I rarely type something like
`(use-modules (srfi srfi-60))' _inadvertently_ in a Scheme file.
Additionally, the _documentation_ of that module already warned me about
the introduction of a different `bit-count' procedure, so if something
goes wrong, it's all my fault.  ;-)

See also:
http://lists.gnu.org/archive/html/guile-devel/2005-10/msg00101.html .

> If you don't want to warning, you can define your own way of handling
> duplicates.  See the NEWS file for docs about the options.

Precisely, you said you were ok to apply the following documentation
yesterday:

  This is useful for modules that export bindings that have the same
  name as core bindings.  @code{#:replace}, in a sense, lets Guile know
  that the module @emph{purposefully} replaces a core binding.  It is
  important to note, however, that this binding replacement is confined
  to the name space of the module user.  In other words, the value of the
  core binding in question remains unchanged for other modules.

So, do we agree on what `#:replace' is for?  :-)

As a matter of fact, this facility had remained undocumented for years
and its original author is no longer here (I think) to explain the
rationale behind it.

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] SRFI-34, SRFI-60 and core bindings

[Marius Vollmer]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> See also:
> http://lists.gnu.org/archive/html/guile-devel/2005-10/msg00101.html .
>
>> If you don't want to warning, you can define your own way of handling
>> duplicates.  See the NEWS file for docs about the options.
>
> Precisely, you said you were ok to apply the following documentation
> yesterday:
>
>   This is useful for modules that export bindings that have the same
>   name as core bindings.  @code{#:replace}, in a sense, lets Guile know
>   that the module @emph{purposefully} replaces a core binding.  It is
>   important to note, however, that this binding replacement is confined
>   to the name space of the module user.  In other words, the value of the
>   core binding in question remains unchanged for other modules.
>
> So, do we agree on what `#:replace' is for?  :-)

My understanding of #:replace goes along these lines: Importing more
than one binding with the same name into a module should be avoided,
and we should make it an error.  But since it was traditonally allowed
by Guile, we merely warn about it.

However, sometimes one of the bindings is a useful replacement for the
other in the sense that it is fully compatible with it.  In that
situation, it is a nice service by the module system to automatically
choose the replacement and not give a warning.  In that scenario,
using a module is not only a way of saying what set of names you want
to use, but also a way of saying what level of functionality one wants
to have for a name that might also be available without using that
module.  You can use #:replace to mark those bindings that are useful
replacements in this sense to get rid of the useless warning.

When using #:replace, the exporting module is making the promise that
the warning can be surpressed because everything is alright.  It can
only do that if the replacing binding is compatible with what it
replaces.

Often, the user of a module also knows that everything is alright and
wants to surpress the warning.  (That's where you come in,
Ludovic. :-) The user can only do that by giving parameters to
use-modules or its own define-module.  He should not go and add a
#:replace to the exporting module.  The only ways to do avoid the
duplicate warning for a module user currently are to rename the
bindings on import, or to use an explicit #:duplicates handler.
(Right?  Anything else?)

> As a matter of fact, this facility had remained undocumented for years
> and its original author is no longer here (I think) to explain the
> rationale behind it.

Hmm, I think current usage is in line with what I described above.
Right?

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] SRFI-34, SRFI-60 and core bindings

[Ludovic Courtès]

Marius Vollmer <mvo@zagadka.de> writes:

> When using #:replace, the exporting module is making the promise that
> the warning can be surpressed because everything is alright.  It can
> only do that if the replacing binding is compatible with what it
> replaces.

So I think we disagree.  ;-)  Basically, I'm saying that `#:replace' is
alright when the module /purposefully/ overrides and binding and is
documented to do so, while you say that it should be used only when a
module /compatibly/ overrides a binding.

I still think that one never types `(use-modules (srfi srfi-60))'
inadvertently, which supports the `purposefully' vision.  IOW, the
"common case" (and this is debatable) is that you _want_ `bit-count' to
be overridden when you type this in.  And we want to optimize for the
common case.

In any case, the documentation will have to be updated to whatever
vision you think is right.

> Often, the user of a module also knows that everything is alright and
> wants to surpress the warning.  (That's where you come in,
> Ludovic. :-) The user can only do that by giving parameters to
> use-modules or its own define-module.  He should not go and add a
> #:replace to the exporting module.  The only ways to do avoid the
> duplicate warning for a module user currently are to rename the
> bindings on import, or to use an explicit #:duplicates handler.
> (Right?  Anything else?)

I find this use of duplicate binding policies quite circumvoluted and
unclean for that matter.

> Hmm, I think current usage is in line with what I described above.
> Right?

Right, within Guile.  Outside, current usage may be inexistent because
it was not documented.

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] SRFI-34, SRFI-60 and core bindings

[Neil Jerram]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> Marius Vollmer <mvo@zagadka.de> writes:
>
>> Often, the user of a module also knows that everything is alright and
>> wants to surpress the warning.  (That's where you come in,
>> Ludovic. :-) The user can only do that by giving parameters to
>> use-modules or its own define-module.  He should not go and add a
>> #:replace to the exporting module.  The only ways to do avoid the
>> duplicate warning for a module user currently are to rename the
>> bindings on import, or to use an explicit #:duplicates handler.
>> (Right?  Anything else?)
>
> I find this use of duplicate binding policies quite circumvoluted and
> unclean for that matter.

I agree with Ludovic.  What we have now seems seriously screwy, and
I'm sorry I didn't pay attention when this was first discussed and
implemented.

(duplicate-handlers is a module??!!  And there was me thinking we
already had quite a good class system in Guile.)

It seems completely wrong for the defining module to be responsible
for guessing which of its bindings might have the same name as other
bindings in the wide world outside that module.

In particular, this gives the absurd situation where a module can be
100% correct one day, then we add a new function to core Guile, and
that same module now becomes incorrect.

I'm not saying that it's bad that users of the module suddenly get a
warning in this situation; rather that it's wrong that the only
(practical) way to fix the warning is to update the _module_'s code.
(IMO using duplicate-handlers is far too heavyweight to be practical.)

It strikes me as a much more reasonable proposition for the _using_
module to be responsible for the bindings that it imports, and the
fact that we already have #:select, #:hide and #:renamer is in line
with this.  So it seems obvious that #:replace should really be
another option for use-modules and #:use-module, _not_ for
define-module.

Am I missing something?  I can't see any good arguments for the
current interface.

Regards,
        Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] SRFI-34, SRFI-60 and core bindings

[Ludovic Courtès]

Hi,

Neil Jerram <neil@ossau.uklinux.net> writes:

> It seems completely wrong for the defining module to be responsible
> for guessing which of its bindings might have the same name as other
> bindings in the wide world outside that module.

I disagree.  The set of core bindings is not supposed to be very large,
and it's not that hard to see whether you're overriding a core binding.
Given that, that doesn't seem too much of a burden for the defining
module to explicitly warn its users (via `#:replace') of its intention
to override a core binding.

> In particular, this gives the absurd situation where a module can be
> 100% correct one day, then we add a new function to core Guile, and
> that same module now becomes incorrect.

I don't think this is an issue, practically.  Theoretically, that should
_not_ be an issue because we should try to keep the set of core bindings
as small as possible IMO.

> It strikes me as a much more reasonable proposition for the _using_
> module to be responsible for the bindings that it imports, and the
> fact that we already have #:select, #:hide and #:renamer is in line
> with this.  So it seems obvious that #:replace should really be
> another option for use-modules and #:use-module, _not_ for
> define-module.

In a way, this is already what happens:

1. The defining module provides a /hint/ about purposefully overridden
   bindings, via a `#:replace' option to `define-module';

2. The using module is /free/ to take such hints into account; by
   default, it will do so, because it will be using the `replace'
   duplicate binding handling policy (see the recent additions to the
   manual in HEAD).

My conclusion is that the current interface is actually quite good.  It
just needs to be well documented.  :-)

Thanks, and best wishes!

Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel