bug#19459: #:export does not honor the merge-generics contract

bug#19459: #:export does not honor the merge-generics contract

[David Pirotte]

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

Hello,

	debian testing
	  GNU Guile 2.0.11	[ mine is .10-fa1a3-dirty, but it does not matter
					[ i'm stuck in that version until goops is patched
					[ and compiles guile-gnome again...
	    #:export does not honor the merge-generics contract

#:export should be adapted to honor the 'merge-generics user request and 'contract'.

What ever forces the 'system' to create a generic function, #:export in this case,
when 'merge-generics has been set, the system should or use the imported generic and
add the newly locally defined method, or create a new one [if that is necessary for its
internal machinery] _but_ [immediately] merge it with th imported one, since it is
the user request.

Right now, even with the merge-generic setting, #:export calls
module-ensure-local-variable! before anything else, creates a new get-with
var, first unbound, later turned into a generic function with 1
applicable [locally defined] method only _but_ it does _not_not merge it, leading to "No
applicable method..." bugs, as in the following example [simple and stupid, but it
only partially mimics real case situation, see the image.scm attachment of this mail
if you're interested http://www.mail-archive.com/guile-devel@gnu.org/msg12618.html

Happy hacking,
David

;; module a starts here
(define-module (a)
  #:use-module (oop goops)
  #:export (<a>
	    !width
	    get-width
	    set-width))


(define-class <a> ()
  (width #:accessor !width #:init-keyword #:width #:init-value 0))

(define-method (get-width (self <a>))
  (!width self))

(define-method (set-width (self <a>) width)
  (set! (!width self) width))
;;  module a ends here


;; module b starts here
(define-module (b)
  #:use-module (oop goops)
  #:use-module (a)
  #:export (<b>
	    !width
	    get-width
	    set-width))


(define-class <b> ()
  (width #:accessor !width #:init-keyword #:width #:init-value 0))

(define-method (initialize (self <b>) initargs)
  (next-method)  
  (let ((a (make <a>)))
    (set-width self (get-width a))
    #;(add-child b a)))

(define-method (get-width (self <b>))
  (!width self))

(define-method (set-width (self <b>) width)
  (set! (!width self) width))
;;  module b ends here


scheme@(guile-user)> ,use (oop goops)
scheme@(guile-user)> (default-duplicate-binding-handler '(merge-generics replace warn-override-core warn last))
scheme@(guile-user)> ,use (b)
;;; note: source file ./b.scm
;;;       newer than compiled /home/david/.cache/guile/ccache/2.0-LE-8-2.0/usr/alto/projects/guile-tests/goops/export/b.scm.go
;;; note: auto-compilation is enabled, set GUILE_AUTO_COMPILE=0
;;;       or pass the --no-auto-compile argument to disable.
;;; compiling ./b.scm
;;; compiled /home/david/.cache/guile/ccache/2.0-LE-8-2.0/usr/alto/projects/guile-tests/goops/export/b.scm.go
scheme@(guile-user)> (make <b>)
ERROR: In procedure scm-error:
ERROR: No applicable method for #<<generic> get-width (1)> in call (get-width #<<a> 2b70880>)

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]> ,bt
In oop/goops.scm:
   1553:4  3 (#<procedure 2854c80 at oop/goops.scm:1551:0 (class . initargs)> #<<class> <b> 28cb8…>)
In guile-tests/goops/export/b.scm:
    17:20  2 (#<procedure 2b708e0 at guile-tests/goops/export/b.scm:14:0 (self initargs)> #<<…> …)
In oop/goops/dispatch.scm:
    239:9  1 (cache-miss #<<generic> get-width (1)> (#<<a> 2b70880>))
In unknown file:
           0 (scm-error goops-error #f "No applicable method for ~S in call ~S" (#<<generic> …> …) …)
scheme@(guile-user) [1]> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

bug#19459: #:export does not honor the merge-generics contract

[Andy Wingo]

Hi,

On Sun 28 Dec 2014 19:20, David Pirotte <david@altosw.be> writes:

> (define-module (a)
>   #:use-module (oop goops)
>   #:export (<a>
> 	    !width
> 	    get-width
> 	    set-width))

Here you export four bindings: one class and three generics.  Those
three generics have methods on <a>.

> (define-module (b)
>   #:use-module (oop goops)
>   #:use-module (a)

Here you import the previous four bindings.

>   #:export (<b>
> 	    !width
> 	    get-width
> 	    set-width))

However here you declare that you are going to export four new
bindings.  I believe this is the source of your problem.  You are
expecting to extend the three generics and re-export them.  However to
do so you should #:re-export !width, get-width, and set-width.

AFAIU there is no bug here.  David WDYT?

Andy

bug#19459: #:export does not honor the merge-generics contract

[David Pirotte]

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

Hi Andy,

> > (define-module (a)
> >   #:use-module (oop goops)
> >   #:export (<a>
> > 	    !width
> > 	    get-width
> > 	    set-width))  

> Here you export four bindings: one class and three generics.  Those
> three generics have methods on <a>.

> > (define-module (b)
> >   #:use-module (oop goops)
> >   #:use-module (a)  

> Here you import the previous four bindings.

> >   #:export (<b>
> > 	    !width
> > 	    get-width
> > 	    set-width))  

> However here you declare that you are going to export four new
> bindings.
> ...

Under the exact circumstances of the original email, I disagree, see below.

	From the original email, you only kept the module defs and explains here in your
	answer the expected behavior in normal circumstances: the behavior one can
	expect from the default Guile configuration. Fine, but that's not the what
	the original email was complaining about :)

> AFAIU there is no bug here.  David WDYT?

IMO it is a bug, and to be honest, IMO it is a serious one: a user should never have
to use #:re-export for generic functions once he/she did ask to merge duplicate
generics, because under this setting, there can be 1 and only 1 generic function, at
any time in any module. Under these circumstances, it is the generic function as the
module 'sees it' that the user export, not the generic function as the module sees
it before import(s)

	if imported, then that generic is 'filled in' with new methods, it is _not_
	created [should not be created], hence #:export is the 'culprit', because as
	it is it does not look if a generic exists and create a new one arbitrarily,
	against the user 'wish'. It can only do so if there is no imported one.

So, IMO, under the '(merge-generics ...) setting of the original email (b) exports 1
new binding and 3 generic functions that were already defined by (a), but/and
'filled' with 3 additional methods. The module (b) exports the generic function 'as
it has it'.

There is another way, maybe, to look at this anomaly: if you comment the export for
get-width and set-width from the (b) module, it works:

	scheme@(guile-user)> ,use (b)
	scheme@(guile-user)> (make <b>)
	$4 = #<<b> 2051a40>

because within the module (b), the generics set-width and get-width were imported
from (a), not created [or 'immediately' merged, implementation detail...], and filed
with the (b) methods. #:export however dismantle this to export a new generic [but
the user ask to merge them, so it breaks the user 'contract'] only containing the (b)
methods, which is a bug.

Last but not least, when a user imports (b) and run (make <b>), he is not himself
calling set-width or get-width: exported or not, the code being run by make should
run within the (b) module, and in the (b) module the generic functions have been
merged,, so can not [should] raise an error. If you follow your idea/position, it
should only raise an error if I do (get-width (make <a>)) [but then the system might
raise the undefined <a> class first..., but you get the idea I guess.

FIWIW, It's been years that I don't use #:export because of this bug, I defined my
own g-export syntax, based on guile's module source code, here:

	http://git.savannah.gnu.org/cgit/grip.git/tree/grip/g-export.scm

It would be really good if it be fixed in 2.0.12 and master though.

Cheers,
David

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

bug#19459: #:export does not honor the merge-generics contract

[Andy Wingo]

On Thu 23 Jun 2016 21:23, David Pirotte <david@altosw.be> writes:

> Hi Andy,
>
>> > (define-module (a)
>> >   #:use-module (oop goops)
>> >   #:export (<a>
>> > 	    !width
>> > 	    get-width
>> > 	    set-width))  
>
>> Here you export four bindings: one class and three generics.  Those
>> three generics have methods on <a>.
>
>> > (define-module (b)
>> >   #:use-module (oop goops)
>> >   #:use-module (a)  
>
>> Here you import the previous four bindings.
>
>> >   #:export (<b>
>> > 	    !width
>> > 	    get-width
>> > 	    set-width))  
>
>> However here you declare that you are going to export four new
>> bindings.
>> ...
>
> Under the exact circumstances of the original email, I disagree, see below.

I see.  You were expecting for the dynamically setting the
default-duplicate-binding-handler parameter to make a difference.
However I think this is maybe not the right way to set this up; see
reasoning in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20093.

However... I believe merge-generics is intended to merge duplicate
imported bindings.  It does not provide a copy-on-write version of an
imported generic, if that generic was not duplicated in the imports.
There is no facility in GOOPS to do that, AFAIU.

Did I get it right this time? :)

Andy

bug#19459: #:export does not honor the merge-generics contract

[David Pirotte]

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


> > Hi Andy,
> >  
> >> > (define-module (a)
> >> >   #:use-module (oop goops)
> >> >   #:export (<a>
> >> > 	    !width
> >> > 	    get-width
> >> > 	    set-width))    
> >  
> >> Here you export four bindings: one class and three generics.  Those
> >> three generics have methods on <a>.  
> >  
> >> > (define-module (b)
> >> >   #:use-module (oop goops)
> >> >   #:use-module (a)    
> >  
> >> Here you import the previous four bindings.  
> >  
> >> >   #:export (<b>
> >> > 	    !width
> >> > 	    get-width
> >> > 	    set-width))    
> >  
> >> However here you declare that you are going to export four new
> >> bindings.
> >> ...  
> >
> > Under the exact circumstances of the original email, I disagree, see below.  
> 
> I see.  You were expecting for the dynamically setting the
> default-duplicate-binding-handler parameter to make a difference.

Yes, and it has to: with that setting, at any time in any module, there can be 1
and only 1 generic function for a given name:

	but as it is, right now, even with that setting, Guile is exporting
	'something' I did not ask to export, Guile is exporting a new binding,
	that I never asked to create [unless it would not be imported]

	from the (b) module perspective, I ask Guile to import (a), fill the generic
	functions with new methods, and export it, these generic functions, not new
	other ones

The symptom of the actual anomaly is very well expressed by my last sentence:

	"... last but not least, when a user imports (b) and run (make <b>), he is
	not himself calling set-width or get-width: exported or not, the code being
	run by make should run within the (b) module, and in the (b) module they can
	be only 1 generic function, with the method of (a) and the method of (b)..."

This should never raise an exception:

	scheme@(guile-user)> ,use (b)
	scheme@(guile-user)> (make <b>)

Currently it does if the user used #:export, it does not if he/she does not export

The problem is guile's module system, not GOOPS here

> However I think this is maybe not the right way to set this up; see
> reasoning in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20093.

	That is/was a totally different bug, but thanks for the fix! I disagree with
	the comment though, but I will pull, compile test it will take 1 or 2
	days... I hope it fixes it!

> However... I believe merge-generics is intended to merge duplicate
> imported bindings.  It does not provide a copy-on-write version of an
> imported generic, if that generic was not duplicated in the imports.
> There is no facility in GOOPS to do that, AFAIU.

It is a module bug, not a GOOPS bug, see my 'personal/local' fix: the problem is
that once the user uses #:export, guile's module system create a new binding, and it
should not ... [hence this confusion as well: as it is: the module must merge its
definition with the imported ones, even if it imported only 1 generic ... because of
a module bug...]

David




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

bug#19459: #:export does not honor the merge-generics contract

[Andy Wingo]

On Thu 23 Jun 2016 23:11, David Pirotte <david@altosw.be> writes:

>> However... I believe merge-generics is intended to merge duplicate
>> imported bindings.  It does not provide a copy-on-write version of an
>> imported generic, if that generic was not duplicated in the imports.
>> There is no facility in GOOPS to do that, AFAIU.
>
> It is a module bug, not a GOOPS bug, see my 'personal/local' fix: the problem is
> that once the user uses #:export, guile's module system create a new binding, and it
> should not ... [hence this confusion as well: as it is: the module must merge its
> definition with the imported ones, even if it imported only 1 generic ... because of
> a module bug...]

I... I just think you're wrong here, sorry :/ That's just not how the
system works.  If you #:export an identifier in a module, you create a
fresh local binding, and that binding doesn't implicitly extend an
imported binding, merge-generics or no.  Merge-generics only operates on
the import interface of a module.

Andy

bug#19459: #:export does not honor the merge-generics contract

[David Pirotte]

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

Hello Andy,

> >> However... I believe merge-generics is intended to merge duplicate
> >> imported bindings.  It does not provide a copy-on-write version of an
> >> imported generic, if that generic was not duplicated in the imports.
> >> There is no facility in GOOPS to do that, AFAIU.  

> > It is a module bug, not a GOOPS bug, see my 'personal/local' fix: the problem is
> > that once the user uses #:export, guile's module system create a new binding,
> > and it should not ... [hence this confusion as well: as it is: the module must
> > merge its definition with the imported ones, even if it imported only 1
> > generic ... because of a module bug...]  

> I... I just think you're wrong here, sorry :/ That's just not how the
> system works.  If you #:export an identifier in a module, you create a
> fresh local binding, and that binding doesn't implicitly extend an
> imported binding, merge-generics or no.  Merge-generics only operates on
> the import interface of a module.

I don't think so, and I feel sorry too ;/. We disagree, which is different, and
nobody is 'right' or 'wrong' here. [and I know 'how the system works, I described
it that in the original email, I'd like to change that, hence this thread ...]

IMO, this should never fail:

	,use (b)
	make <b>

Your last sentence states that merge-generics only operates on the import interface
of a module: that is the feature I'm referring to to claim the above:

  [ using #:export ]

	t0	the system creates new 'empty' binding
		at some point 'it' knows it is a gf
	ti	it imports (a)

	now the (b) module interface is facing a duplicate gf binding, and according
	to the user settings wrt to this, it has to merge.

Then you tell me 'yep David, internally the system faced an import situation, and
ok, let's merge, _but_ #:export must export a fresh new ... I can buy that, but
do we have the technology do implement this senario? 

I believe, still referring to your last sentence, the above sentence as well and
respecting your position, that we should rather create/have global setting
'export-rexport-imported-gf' [or a better name.], WDYT?

Happy Hacking,
David

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

bug#19459: #:export does not honor the merge-generics contract

[Andy Wingo]

On Mon 27 Jun 2016 04:54, David Pirotte <david@altosw.be> writes:

> Hello Andy,
>
>> >> However... I believe merge-generics is intended to merge duplicate
>> >> imported bindings.  It does not provide a copy-on-write version of an
>> >> imported generic, if that generic was not duplicated in the imports.
>> >> There is no facility in GOOPS to do that, AFAIU.  
>
>> > It is a module bug, not a GOOPS bug, see my 'personal/local' fix: the problem is
>> > that once the user uses #:export, guile's module system create a new binding,
>> > and it should not ... [hence this confusion as well: as it is: the module must
>> > merge its definition with the imported ones, even if it imported only 1
>> > generic ... because of a module bug...]  
>
>> I... I just think you're wrong here, sorry :/ That's just not how the
>> system works.  If you #:export an identifier in a module, you create a
>> fresh local binding, and that binding doesn't implicitly extend an
>> imported binding, merge-generics or no.  Merge-generics only operates on
>> the import interface of a module.
>
> I don't think so, and I feel sorry too ;/. We disagree, which is different, and
> nobody is 'right' or 'wrong' here. [and I know 'how the system works, I described
> it that in the original email, I'd like to change that, hence this thread ...]
>
> IMO, this should never fail:
>
> 	,use (b)
> 	make <b>

There are plenty of reasons for (make foo) to fail in the abstract --
from bad logic in the initializers, to the initializer using unbound
variables, to many other things.  In this case your initializer is:

(define-method (initialize (self <b>) initargs)
  (next-method)  
  (let ((a (make <a>)))
    (set-width self (get-width a))
    #;(add-child b a)))

However in this module you have done an #:export get-width on B, and so
the get-width that you are calling in module (b) has no methods for
values of type A.

> Your last sentence states that merge-generics only operates on the import interface
> of a module: that is the feature I'm referring to to claim the above:
>
>   [ using #:export ]
>
> 	t0	the system creates new 'empty' binding
> 		at some point 'it' knows it is a gf
> 	ti	it imports (a)
>
> 	now the (b) module interface is facing a duplicate gf binding, and according
> 	to the user settings wrt to this, it has to merge.

I'm not sure from this whether you think the current code has a bug or a
feature limitation.

For me :) I have it clear in my mind that there does not appear to be a
bug here.  merge-generics only operates over bindings that are imported
in a module: module A imports modules B and C, and B and C both export a
generic `foo'.  Or one is a generic and one is not, or something like
that.  merge-generics *does not* merge an imported binding and a local
definition.

It does seem like you want the system to work in a different way, and
that position is understandable -- however designing and implementing
such a thing is very low on my priority list right now and I doubt I
will get to it any time soon if at all.  If you or someone else wants to
implement this, the first step would be to come up with a design
including a patch to the documentation and mail the list.  I think I
would want the design before seeing a patch.

Regards,

Andy