Guile-2.2 - goops setters should be inherited, no matter what :)

Guile-2.2 - goops setters should be inherited, no matter what :)

[David Pirotte]

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

Hello Andy,

>  * GOOPS: are there incompatible changes that we think are bad?
>    Subthread :)  

	In my opinion, this would be the right time to make changes so that goops follows
	the clos protocol (for the (quite large) subset it implements), I don't
	pretend I have identified all 'breaches', but two are essential to me, here
	is the first (I'll post the other in another thread).

1-	setters, as in (define-method ((setter ...) (self <...>) ...) ...)
	should (also :)) be inherited,

	_whether or not_ the user defined an accessor
	
	[ see bug#19770
	[ so I can assume for this discussion we have 2 modules, (a) and (b) as
	[ defined i that bug report

Now, wrt this problem, our manual seems to follow the clos protocol, but not our
implementation. Here is what our manual says:

	... If generic is (setter generic-with-setter), where generic-with-setter is
	a  variable  which is not yet bound to a generic-with-setter object, the
	expansion will include  a call to define-accessor...

So, even if I comment the accessor definition in the module (a), the expansion should
create one, and <b> should inherit it: in both cases an accessor is created and <b>
should inherit it, as well as the setter, but that is not what happens in our implementation.

Here is what happens if I comment the accesor defintion in (a):

	Guile 2.1.7.10-f261eaf
	scheme@(guile-user)> ,use (oop goops)
	scheme@(guile-user)> ,use (b)
	scheme@(guile-user)> (make <b>)
	$2 = #<<b> 558f91331260>
	scheme@(guile-user)> (!width $2)
	ERROR: In procedure scm-error:
	ERROR: No applicable method for #<<accessor> !width (0)> in call (!width #<<b> 558f91331260>)

	Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
	scheme@(guile-user) [1]> ,q

_but_ the setter has been inherited:

	scheme@(guile-user)> (set! (!width $2) 20)

	;;; ("this is <a> !width setter method, hello!")
	$3 = 20
	scheme@(guile-user)> 

That is quite inconsistent imo. I think  we should 'clear' this up and make things
work as the protocol and our manual says:

	- if an accessor was not define, the seteer definition expansion should
	  define one;
	
	- we should not 'force' the user to _not define_ the accessor him/her
	  self, he/she should perfectly be allowed to do so (I recommend it
	  actually);

	- both the accessor and the setter should be inherited

WDYT?
David

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

Re: Guile-2.2 - goops setters should be inherited, no matter what :)

[Christopher Allan Webber]

David Pirotte writes:

> 	- both the accessor and the setter should be inherited

I've run into unexpected issues where I redefined a slot and didn't
remember to set the same getter/setter/accessor.  I'd also love to see
that be an inherited.  It would lead to less goofups in my code.

But why just setters and accessors?  Why not also getters?

  scheme@(guile-user)> (use-modules (oop goops))
  scheme@(guile-user)> (define-class <forp> ()
                         (da-slot #:init-keyword #:da-slot
                                  #:getter get-da-slot))
  scheme@(guile-user)> (define-class <borp> (<forp>)
                         (da-slot #:init-value "moop"))
  scheme@(guile-user)> (get-da-slot (make <borp>))
  ERROR: In procedure scm-error:
  ERROR: No applicable method for #<<generic> get-da-slot (1)> in call (get-da-slot #<<borp> 22180a0>)

  Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
  scheme@(guile-user) [1]> ,q

It may also be worth asking, what other slot fields get inherited?
Would #:init-keyword?  #:init-value?  #:init-thunk?  Inheriting
#:init-value and #:init-thunk both could be confusing (what does CLOS
do?).  What about #:allocation?

Re: Guile-2.2 - goops setters should be inherited, no matter what :)

[David Pirotte]

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

Hi Christopher,

	You are confusing/mixing two different issues, precisely the two issues I'm
	talking about in my two separate emails, but you mixed them here :)

	I'd rather keep them well separated, because fixing the first should be very
	easy, it is a matter of agreeing (and a bit of work of course, adapting the
	test suite... I'm not saying it is zero work :)), where as fixing the
	second, slot redefinition at subclass to follow the clos protocol, represent
	quite a lot of work, so it is not just a matter of agreeing...

> > 	- both the accessor and the setter should be inherited  
> 
> I've run into unexpected issues where I redefined a slot and didn't
> remember to set the same getter/setter/accessor.

As things are, both in 2.0 and 2.2, if you redefine a slot, it is a new slot, nothing
is inherited (wrt to the slot I mean, methods are of course):

	but this refers to my other email, let's keep the issues separated

> But why just setters and accessors?  Why not also getters?

As I said above, When you do _not_ redefine the slot at subclass level, everything
is inherited, except setters, hence my original bug report and this fresh email
which contains a much better explanation/situation description for Andy to 'play'
with ...

> It may also be worth asking, what other slot fields get inherited?
> Would #:init-keyword?  #:init-value?  #:init-thunk?  Inheriting
> #:init-value and #:init-thunk both could be confusing (what does CLOS
> do?).  What about #:allocation?

This refers to the other issue, and what clos does is in the summary of that email,
as well as in the original bug report, which has links to further read ... of course
the full protocol is in the spec

Cheers,
David

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

Re: Guile-2.2 - goops setters should be inherited, no matter what :)

[Andy Wingo]

Hi,

On Sun 26 Feb 2017 23:57, David Pirotte <david@altosw.be> writes:

> 1-	setters, as in (define-method ((setter ...) (self <...>) ...) ...)
> 	should (also :)) be inherited,

As you mention this is https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19770.

I think we have an understanding about why things are the way they are
in GOOPS, and you are arguing that they should be different -- different
from 1.8 and 2.0.  That's OK and making a different system is possible
if we have good reasons.  It's also possible to extend the current
system to implement new behaviors.

In this case though I don't know how to make a consistent system with
the semantics you are looking for and without losing some of the speed
of the current system.  I guess you would want for the class defining
the slot to define a method that just does (slot-set! obj 'slot x), and
not have concrete subclasses define their own accessor methods, thereby
avoiding accessors entirely.  In that case I would think you could
define slots with a different kind of class, or override the slot
definition protocol or something, or use a different define-class
wrapper or something.

In short I think I just don't agree with this change as part of standard
GOOPS, so I propose the second solution: to make sure you can implement
the behavior you want as a user.  What about using a wrapper
define-class macro that removes "#:accessor foo" from its slot
definitions and translates those to issue definitions like this:

  (define-method (foo (x <obj>)) (slot-ref x 'foo))
  (define-method ((setter foo) (x <obj>) val) (slot-set! x 'foo val))

Andy

Re: Guile-2.2 - goops setters should be inherited, no matter what :)

[David Pirotte]

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

Hello Andy,

> > 1-	setters, as in (define-method ((setter ...) (self <...>) ...) ...)
> > 	should (also :)) be inherited

> As you mention this is https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19770.

Yep, but after carefully (re)reading your answer and the manual, doing some more
tests, I came to the conclusion stated in my message (the newly sent message), which
in summary says:

	our documentation says it will create the accessor, unless already created
	=> this, to me, means that:

	(a) if the expansion code creates the accessor, it should be possible for
	users to use it on both <a> and <b> instances;

	(b) from  the text of the manual itself, the user should be able to create
	the accessor him/herself, then redefine the (setter foo) method _without_
	altering any semantic wrt to the situation users faces when they are in the
	(a) situation

In other words, once a user defines a setter, both the setter and getter (auto
created or not) should be available upon the class they belong to and all subclasses

> I think we have an understanding about why things are the way they are
> in GOOPS ...

Good! And I understand you mostly have, if ot entirely  inherited and not redefined
the <situation>	:)

> In this case though I don't know how to make a consistent system with
> the semantics you are looking for and without losing some of the speed
> of the current system.

I'm sorry to hear that, but then we should change the manual, since goops does not
implement what is described... 

	note that on a semantic ground, it does not make much sense to define a
	setter upon a inexistent 'variable location', so, to me, it makes more
	sense to implement what the manual says;

	I would definitely not consider speed here, not again semantic anyway, but
	instead implement what the manual says, maybe with a special note in the
	manual to explain, if there is?, a way to achieve the same result with better
	speed... (like using #:getter then define-method (setter foo) ... ?)

> In short I think I just don't agree with this change as part of standard
> GOOPS, so I propose the second solution: to make sure you can implement
> the behavior you want as a user.  What about using a wrapper define-class macro
> that removes "#:accessor foo" from its slot definitions and translates those to
> issue definitions like this:
> 
>   (define-method (foo (x <obj>)) (slot-ref x 'foo))
>   (define-method ((setter foo) (x <obj>) val) (slot-set! x 'foo val))

Sure.  Do you mean making such a wrapper available in 2.2.x (as opposed to each of
us would have to write his own?

By the way that is exactly what the clos protocol says defclass should be expanded
to when using :accessor, see [2] for an sbcl session upon the same 'case' (adapted
for syntax and argument order (setf method first argument _must_ be the value ...))

	just in case you'd like to play with again, I have made the example code even
	simpler see [1] (comment/uncomment the definition of the slot with/without
	the #:accessor   and reload, to see how guile 'behaves' in both case...


To make it short :):), I'm fine to use the second solution, though allow me to
insist upon the fact that it is 'weird', to me, that the actual implementation
properly inherits the setter if the user does not define the accessor, but then
there is no getter, and if the user defines the accessor, then the setter is not
inherited anymore.


Cheers,
David

[1]

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

(define-method ((setter width) (self <a>) val)
  ;; here comes complex code, computing earth orbit, captain's age...
  (pk "this is <a> !width setter method, hello!")
  (slot-set! self 'width val)
  val)

(define-class <b> (<a>))

(define a1 (make <a>))
(define b1 (make <b>))

(set! (width a1) 10)
(width a1)

(set! (width b1) 10)
(width b1)

[2]

(defclass <a> ()
  ((width :accessor width :initform 0)))

(defmethod (setf width) (val (self <a>))
  ;; here comes complex code, computing earth orbit, captain's age...
  (format t "this is <a> (setf width) method, hello!")
  (setf (slot-value self 'width) val)
  val)

(defclass <b> (<a>) ())

(defvar a1 (make-instance '<a>))
(defvar b1 (make-instance '<b>))

=>

* (setf (width a1) 10)
this is <a> (setf width) method, hello!
10
* (width a1)

10
* (setf (width b1) 10)
this is <a> (setf width) method, hello!
10
* (width b1)

10
* 

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

Re: Guile-2.2 - goops setters should be inherited, no matter what :)

[David Pirotte]

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

Le Fri, 17 Mar 2017 05:46:47 -0300,
David Pirotte <david@altosw.be> a écrit :

> Hello Andy,
> ...

Sorry for the duplicate email, my bad!
David

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