Keywords in GOOPS methods

Keywords in GOOPS methods

[Mikael Djurfeldt]

[-- Attachment #1.1: Type: text/plain, Size: 407 bytes --]

Hi all,

I've implemented support for keyword arguments (corresponding to define*
and lambda*) in GOOPS. The functionality is similar to that of CLOS (which
also has keyword in methods) in that dispatch is not done on the keyword
part.

You can find the changes in the goops-keyword branch at
https://github.com/mdjurfeldt/guile/tree/goops-keyword or in the included
patch.

Comments?

Best regards,
MIkael

[-- Attachment #1.2: Type: text/html, Size: 642 bytes --]

[-- Attachment #2: goops-kw-patch.patch --]
[-- Type: text/x-patch, Size: 11808 bytes --]

diff --git a/module/oop/goops.scm b/module/oop/goops.scm
index 8ed68694c..c0490c84a 100644
--- a/module/oop/goops.scm
+++ b/module/oop/goops.scm
@@ -135,7 +135,7 @@
             class-slots
             generic-function-name
             generic-function-methods method-generic-function
-            method-specializers method-formals
+            method-specializers method-formals method-keyword-formals?
             primitive-generic-generic enable-primitive-generic!
             method-procedure accessor-method-slot-definition
             make find-method get-keyword))
@@ -1052,6 +1052,7 @@ slots as we go."
   specializers
   procedure
   formals
+  keyword-formals?
   body
   make-procedure)
 (define-standard-class <accessor-method> (<method>)
@@ -1156,6 +1157,7 @@ function."
                     (#:specializers specializers ())
                     (#:procedure procedure #f)
                     (#:formals formals ())
+                    (#:keyword-formals? keyword-formals? #f)
                     (#:body body ())
                     (#:make-procedure make-procedure #f))))
        ((memq <class> (class-precedence-list class))
@@ -2018,14 +2020,14 @@ function."
        (else
         (let lp ((specs specs) (types types))
           (cond
-           ((null? specs) (null? types))
+           ((null? specs)
+            (or (null? types) (method-keyword-formals? m)))
            ((not (pair? specs)) #t)
            ((null? types) #f)
            (else
             (and (memq (car specs) (class-precedence-list (car types)))
                  (lp (cdr specs) (cdr types))))))))))
-  (let ((n (length args))
-        (types (map class-of args)))
+  (let ((types (map class-of args)))
     (let lp ((methods (generic-function-methods gf))
              (applicable '()))
       (if (null? methods)
@@ -2066,8 +2068,27 @@ function."
 
 (define-syntax method
   (lambda (x)
-    (define (parse-args args)
-      (let lp ((ls args) (formals '()) (specializers '()))
+    ;; parse-formals METHOD-FORMALS
+    ;;
+    ;; return (FORMALS SPECIALIZERS KEYWORD-FORMALS)
+    ;;
+    ;; FORMALS is the possibly improper list of specializable formals.
+    ;;
+    ;; SPECIALIZERS is a proper list of the corresponding specializers.
+    ;; Its last element corresponds to the cdr of the last element in
+    ;; METHOD-FORMALS such that the possibly improper list corresponding
+    ;; to FORMALS can be obtained by applying cons* to SPECIALIZERS.
+    ;; The reason for handling it like this is that the specializers are
+    ;; each evaluated to their values and therefore *must* be provided
+    ;; by a cons* in the (make <method> ...) expression.
+    ;;
+    ;; KEYWORD_FORMALS is the part of METHOD-FORMALS which starts with a
+    ;; keyword and corresponds to the keyword-syntax of lambda*. These
+    ;; are not specializable (which also corresponds to CLOS
+    ;; functionality).
+    ;;
+    (define (parse-formals method-formals)
+      (let lp ((ls method-formals) (formals '()) (specializers '()))
         (syntax-case ls ()
           (((f s) . rest)
            (and (identifier? #'f) (identifier? #'s))
@@ -2079,13 +2100,21 @@ function."
            (lp #'rest
                (cons #'f formals)
                (cons #'<top> specializers)))
+          ((f . rest)
+           (keyword? (syntax->datum #'f))
+           (list (reverse formals)
+                 (reverse (cons #''() specializers)) ;to be cons*:ed
+                 (cons #'f #'rest)))
+
           (()
            (list (reverse formals)
-                 (reverse (cons #''() specializers))))
+                 (reverse (cons #''() specializers))
+                 '()))                  ;yes, not #''(); used in tests
           (tail
            (identifier? #'tail)
            (list (append (reverse formals) #'tail)
-                 (reverse (cons #'<top> specializers)))))))
+                 (reverse (cons #'<top> specializers))
+                 '())))))
 
     (define (find-free-id exp referent)
       (syntax-case exp ()
@@ -2098,43 +2127,87 @@ function."
            (and (free-identifier=? #'x id) id)))
         (_ #f)))
 
-    (define (compute-procedure formals body)
+    (define (compute-procedure formals keyword-formals body)
       (syntax-case body ()
         ((body0 ...)
-         (with-syntax ((formals formals))
-           #'(lambda formals body0 ...)))))
-
-    (define (->proper args)
-      (let lp ((ls args) (out '()))
+         (let ((formals (if (null? keyword-formals)
+                            formals
+                            (append formals keyword-formals))))
+           (with-syntax ((formals formals))
+             #`(lambda* formals body0 ...))))))
+
+    ;; ->formal-ids FORMALS
+    ;;
+    ;; convert FORMALS into formal-ids format, which is a cell where the
+    ;; car is the list of car:s in FORMALS and the cdr is the cdr of the
+    ;; last cell in FORMALS.
+    ;;
+    ;; The motivation for this format is to determine at low cost if
+    ;; FORMALS is improper or not and to easily be able to generate the
+    ;; corresponding next-method call.
+    ;;
+    (define (->formal-ids formals)
+      (let lp ((ls formals) (out '()))
+        (syntax-case ls ()
+          ((x . xs) (lp #'xs (cons #'x out)))
+          (()       (cons (reverse out) '()))
+          (tail     (cons (reverse out) #'tail)))))
+
+    ;; keyword-formal-ids KEYWORD-FORMALS
+    ;;
+    ;; return a form corresponding to KEYWORD-FORMALS but with
+    ;; identifiers only (keywords removed) The value returned has the
+    ;; formals-ids format as described above.
+    ;;
+    ;; The output is used in the next-method application form.
+    ;;
+    (define (->keyword-formal-ids keyword-formals)
+      (let lp ((ls keyword-formals) (out '()))
         (syntax-case ls ()
-          ((x . xs)        (lp #'xs (cons #'x out)))
-          (()              (reverse out))
-          (tail            (reverse (cons #'tail out))))))
+          (((f val) . rest)
+           (lp #'rest out))
+          ((#:rest f)
+           (cons (reverse out) #'f))
+          ((f . rest)
+           (keyword? (syntax->datum #'f))
+           (lp #'rest out))
+          ((f . rest)
+           (lp #'rest (cons #'f out)))
+          (()
+           (cons (reverse out) '()))
+          (tail
+           (cons (reverse out) #'tail)))))
 
-    (define (compute-make-procedure formals body next-method)
+    (define (compute-make-procedure formals keyword-formals body next-method)
       (syntax-case body ()
         ((body ...)
-         (with-syntax ((next-method next-method))
-           (syntax-case formals ()
-             ((formal ...)
-              #'(lambda (real-next-method)
-                  (lambda (formal ...)
-                    (let ((next-method (lambda args
-                                         (if (null? args)
-                                             (real-next-method formal ...)
-                                             (apply real-next-method args)))))
-                      body ...))))
-             (formals
-              (with-syntax (((formal ...) (->proper #'formals)))
-                #'(lambda (real-next-method)
-                    (lambda formals
-                      (let ((next-method (lambda args
-                                           (if (null? args)
-                                               (apply real-next-method formal ...)
-                                               (apply real-next-method args)))))
+         (let ((formals (if (null? keyword-formals)
+                            formals     ;might be improper
+                            (append formals keyword-formals)))
+               (formal-ids
+                (if (null? keyword-formals)
+                    (->formal-ids formals)
+                    (let ((kw-formal-ids (->keyword-formal-ids keyword-formals)))
+                      ;; input and result in formals-ids format
+                      (cons (append formals (car kw-formal-ids))
+                            (cdr kw-formal-ids))))))
+           (with-syntax ((next-method next-method))
+             (syntax-case formals ()
+               (formals
+                #`(lambda (real-next-method)
+                    (lambda* formals
+                      (let ((next-method
+                             (lambda args
+                               (if (null? args)
+                                   #,(if (null? (cdr formal-ids))
+                                         #`(real-next-method #,@(car formal-ids))
+                                         #`(apply real-next-method
+                                                  #,@(car formal-ids)
+                                                  #,(cdr formal-ids)))
+                                   (apply real-next-method args)))))
                         body ...))))))))))
 
-    (define (compute-procedures formals body)
+    (define (compute-procedures formals keyword-formals body)
       ;; So, our use of this is broken, because it operates on the
       ;; pre-expansion source code. It's equivalent to just searching
       ;; for referent in the datums. Ah well.
@@ -2142,23 +2215,31 @@ function."
         (if id
             ;; return a make-procedure
             (values #'#f
-                    (compute-make-procedure formals body id))
-            (values (compute-procedure formals body)
+                    (compute-make-procedure formals keyword-formals body id))
+            (values (compute-procedure formals keyword-formals body)
                     #'#f))))
 
     (syntax-case x ()
-      ((_ args) #'(method args (if #f #f)))
-      ((_ args body0 body1 ...)
-       (with-syntax (((formals (specializer ...)) (parse-args #'args)))
+      ((_ formals) #'(method formals (if #f #f)))
+      ((_ formals body0 body1 ...)
+       (with-syntax (((formals (specializer ...) keyword-formals)
+                      (parse-formals #'formals)))
          (call-with-values
              (lambda ()
-               (compute-procedures #'formals #'(body0 body1 ...)))
+               (compute-procedures #'formals
+                                   #'keyword-formals
+                                   #'(body0 body1 ...)))
            (lambda (procedure make-procedure)
              (with-syntax ((procedure procedure)
                            (make-procedure make-procedure))
-               #'(make <method>
-                   #:specializers (cons* specializer ...)
-                   #:formals 'formals
+               #`(make <method>
+                   #:specializers (cons* specializer ...) ;yes, this
+                   ;; The cons* is needed to get at the value of each
+                   ;; specializer.
+                   #:formals (if (null? 'keyword-formals)
+                                 'formals ;might be improper
+                                 (append 'formals 'keyword-formals))
+                   #:keyword-formals? (not (null? 'keyword-formals))
                    #:body '(body0 body1 ...)
                    #:make-procedure make-procedure
                    #:procedure procedure)))))))))
@@ -2281,6 +2362,9 @@ function."
 (define-method (method-formals (m <method>))
   (slot-ref m 'formals))
 
+(define-method (method-keyword-formals? (m <method>))
+  (slot-ref m 'keyword-formals?))
+
 ;;;
 ;;; Slots
 ;;;
@@ -2834,6 +2918,7 @@ var{initargs}."
   (slot-set! method 'procedure
              (get-keyword #:procedure initargs #f))
   (slot-set! method 'formals (get-keyword #:formals initargs '()))
+  (slot-set! method 'keyword-formals? (get-keyword #:keyword-formals? initargs #f))
   (slot-set! method 'body (get-keyword #:body initargs '()))
   (slot-set! method 'make-procedure (get-keyword #:make-procedure initargs #f)))
 

Re: Keywords in GOOPS methods

[Mikael Djurfeldt]

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

Since there are no comments, I'm inclined to apply this patch. I will do
that on Sunday if there are no comments before that.

Maybe I should first give a motivating example: guile-mqtt consists of a
thin wrapper over NYACC-generated code. There, I use a coding pattern that
I tend to use in cases where methods need keyword arguments:

(define-method (subscribe (client <mosquitto-client>) (topic <string>) .
args)
  (define* (subscribe #:key (qos 0))
    (mosquitto_subscribe (mosq client) %null-pointer topic qos))
  (apply subscribe args))

With the change I propose, this can be written:

(define-method (subscribe (client <mosquitto-client>) (topic <string>)
#:key (qos 0))
  (mosquitto_subscribe (mosq client) %null-pointer topic qos)))

with the same resulting semantics.

There is one question that I'd like people to think about, though: In my
patch I have adhered to the close relationship with CLOS, where defmethod
takes keyword, optional and rest arguments similar to Guile's define*, and
extended the method syntax itself. As an alternative, we could let the
current method syntax stay as is and implement new define-method* and
method* syntax. In some ways this would be cleaner, for example from a
backward compatibility perspective. On the other hand it might feel like
overkill to have so much syntax. Implementation and performance wise it
shouldn't matter much how we choose to do, except that adding
define-method* and method* of course adds more code to the implementation...

Best regards,
Mikael

On Tue, Nov 19, 2024 at 5:41 PM Mikael Djurfeldt <mikael@djurfeldt.com>
wrote:

> Hi all,
>
> I've implemented support for keyword arguments (corresponding to define*
> and lambda*) in GOOPS. The functionality is similar to that of CLOS (which
> also has keyword in methods) in that dispatch is not done on the keyword
> part.
>
> You can find the changes in the goops-keyword branch at
> https://github.com/mdjurfeldt/guile/tree/goops-keyword or in the included
> patch.
>
> Comments?
>
> Best regards,
> MIkael
>
>

[-- Attachment #2: Type: text/html, Size: 2796 bytes --]

Re: Keywords in GOOPS methods

[Mikael Djurfeldt]

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

(I will of course add proper documentation in the manual, etc.)

On Thu, Nov 21, 2024 at 9:33 PM Mikael Djurfeldt <mikael@djurfeldt.com>
wrote:

> Since there are no comments, I'm inclined to apply this patch. I will do
> that on Sunday if there are no comments before that.
>
> Maybe I should first give a motivating example: guile-mqtt consists of a
> thin wrapper over NYACC-generated code. There, I use a coding pattern that
> I tend to use in cases where methods need keyword arguments:
>
> (define-method (subscribe (client <mosquitto-client>) (topic <string>) .
> args)
>   (define* (subscribe #:key (qos 0))
>     (mosquitto_subscribe (mosq client) %null-pointer topic qos))
>   (apply subscribe args))
>
> With the change I propose, this can be written:
>
> (define-method (subscribe (client <mosquitto-client>) (topic <string>)
> #:key (qos 0))
>   (mosquitto_subscribe (mosq client) %null-pointer topic qos)))
>
> with the same resulting semantics.
>
> There is one question that I'd like people to think about, though: In my
> patch I have adhered to the close relationship with CLOS, where defmethod
> takes keyword, optional and rest arguments similar to Guile's define*, and
> extended the method syntax itself. As an alternative, we could let the
> current method syntax stay as is and implement new define-method* and
> method* syntax. In some ways this would be cleaner, for example from a
> backward compatibility perspective. On the other hand it might feel like
> overkill to have so much syntax. Implementation and performance wise it
> shouldn't matter much how we choose to do, except that adding
> define-method* and method* of course adds more code to the implementation...
>
> Best regards,
> Mikael
>
> On Tue, Nov 19, 2024 at 5:41 PM Mikael Djurfeldt <mikael@djurfeldt.com>
> wrote:
>
>> Hi all,
>>
>> I've implemented support for keyword arguments (corresponding to define*
>> and lambda*) in GOOPS. The functionality is similar to that of CLOS (which
>> also has keyword in methods) in that dispatch is not done on the keyword
>> part.
>>
>> You can find the changes in the goops-keyword branch at
>> https://github.com/mdjurfeldt/guile/tree/goops-keyword or in the
>> included patch.
>>
>> Comments?
>>
>> Best regards,
>> MIkael
>>
>>

[-- Attachment #2: Type: text/html, Size: 3243 bytes --]

Re: Keywords in GOOPS methods

[janneke]

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

Mikael Djurfeldt writes:

Hi Mikael,

> Since there are no comments, I'm inclined to apply this patch. I will do
> that on Sunday if there are no comments before that.

Just FYI, have you seen the attached patch by Mark Weaver?  I've been
carrying that for some time now, functionality like this is very
welcome.

Greetings,
Janneke


[-- Attachment #2: 0001-Add-define-method-for-GOOPS.patch --]
[-- Type: text/x-patch, Size: 2787 bytes --]

From f4d73b5590cd2e6ff30341aa824a3f1005b50220 Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Fri, 15 Apr 2022 11:11:16 +0200
Subject: [PATCH] Add define-method* for GOOPS.
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=UTF-8

* dzn/define-method-star.scm: New file.
* dzn/local.mk (dist_%C%_scm_DATA): Add it.
---
 dzn/define-method-star.scm | 40 ++++++++++++++++++++++++++++++++++++++
 dzn/local.mk               |  1 +
 2 files changed, 41 insertions(+)
 create mode 100644 dzn/define-method-star.scm

diff --git a/dzn/define-method-star.scm b/dzn/define-method-star.scm
new file mode 100644
index 0000000000..a958907e35
--- /dev/null
+++ b/dzn/define-method-star.scm
@@ -0,0 +1,40 @@
+;;; Dezyne --- Dezyne command line tools
+;;;
+;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
+;;;
+;;; This file is part of Dezyne.
+;;;
+;;; Dezyne is free software: you can redistribute it and/or modify it
+;;; under the terms of the GNU Affero General Public License as
+;;; published by the Free Software Foundation, either version 3 of the
+;;; License, or (at your option) any later version.
+;;;
+;;; Dezyne is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;;; Affero General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU Affero General Public
+;;; License along with Dezyne.  If not, see <http://www.gnu.org/licenses/>.
+;;;
+;;; Commentary:
+;;;
+;;; Code:
+
+(define-module (8sync define-method-star)
+  #:use-module (oop goops)
+  #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-11)
+  #:export (define-method*))
+
+(define-syntax define-method*
+  (lambda (x)
+    (syntax-case x ()
+      ((_ (generic arg-spec ... . tail) body ...)
+       (let-values (((required-arg-specs other-arg-specs)
+                     (break (compose keyword? syntax->datum)
+                            #'(arg-spec ...))))
+         #`(define-method (generic #,@required-arg-specs . rest)
+             (apply (lambda* (#,@other-arg-specs . tail)
+                      body ...)
+                    rest)))))))
diff --git a/dzn/local.mk b/dzn/local.mk
index a6cef16408..99d84ae105 100644
--- a/dzn/local.mk
+++ b/dzn/local.mk
@@ -29,6 +29,7 @@ dist_%C%_scm_DATA =				\
  %D%/code.scm					\
  %D%/code-util.scm				\
  %D%/command-line.scm				\
+ %D%/define-method-star.scm			\
  %D%/display.scm				\
  %D%/explore.scm				\
  %D%/goops.scm					\
-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com


[-- Attachment #3: Type: text/plain, Size: 164 bytes --]


-- 
Janneke Nieuwenhuizen <janneke@gnu.org>  | GNU LilyPond https://LilyPond.org
Freelance IT https://www.JoyOfSource.com | Avatar® https://AvatarAcademy.com

Re: Keywords in GOOPS methods

[Mikael Djurfeldt]

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

Hi Janneke,

That was elegant. :-)

Nope---haven't seen it. (Or, at least I do not remember it.)

Maybe I should have a look at what the optimizer can do about Mark's code.
(As you might have seen, my code is a modification of the method syntax
implementation itself.)

Any opinions on what is best: Having a define-method* or having the
functionality in define-method itself?

Best regards,
Mikael

On Thu, Nov 21, 2024 at 11:00 PM <janneke@gnu.org> wrote:

> Mikael Djurfeldt writes:
>
> Hi Mikael,
>
> > Since there are no comments, I'm inclined to apply this patch. I will do
> > that on Sunday if there are no comments before that.
>
> Just FYI, have you seen the attached patch by Mark Weaver?  I've been
> carrying that for some time now, functionality like this is very
> welcome.
>
> Greetings,
> Janneke
>
>
> --
> Janneke Nieuwenhuizen <janneke@gnu.org>  | GNU LilyPond
> https://LilyPond.org
> Freelance IT https://www.JoyOfSource.com | Avatar®
> https://AvatarAcademy.com
>

[-- Attachment #2: Type: text/html, Size: 1909 bytes --]

Re: Keywords in GOOPS methods

[Tomas Volf]

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

Hello,

opinion from the peanut gallery below.

Mikael Djurfeldt <mikael@djurfeldt.com> writes:

> Any opinions on what is best: Having a define-method* or having the
> functionality in define-method itself?

I do find the symmetry between define-method/define-method* and
define/define* pleasing.

For define and define*, one could argue that procedures produced by the
latter are slower to call (I did measure).  Is that an issue here as
well?  (I guess one could argue that people writing object oriented code
with run-time dispatch are usually not driven by performance as the main
metric.)

You did mention backwards compatibility, but how serious you expect the
issue would be?  I personally did not use GOOPS yet, but I have a hard
time imagining a real-world code that would be broken by this change.
Do you expect there would actually be any?

I personally would probably lean towards two separate procedures (mainly
due to the assumption of there being a performance impact).

Have a nice day,
Tomas Volf
 
-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

Re: Keywords in GOOPS methods

[Mikael Djurfeldt]

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

Hi Tomas,

Thank you for your feedback. Answers below.

On Fri, Nov 22, 2024 at 12:46 PM Tomas Volf <~@wolfsden.cz> wrote:

> I do find the symmetry between define-method/define-method* and
> define/define* pleasing.
>

Yes, I guess we are free to form GOOPS in our own style regardless of CLOS.


> For define and define*, one could argue that procedures produced by the
> latter are slower to call (I did measure).  Is that an issue here as
> well?


Well, the implementation in my patch essentially does the same amount of
work as the old one, and when not using keywords a call is as efficient.
Generic function dispatch should be unaffected, as with Marks macro.
However, there will be the same kind of overhead as for define* in calls
actually using keywords.

I checked what the optimizer can do with the resulting method when using
Marks elegant macro. Unfortunately, such a method would have overhead.
However, I do think that a modified version of Marks macro together with a
small change in peval could have a chance not to introduce such overhead.

All, in all, it currently (and paradoxically) turns out to be simpler to
provide the keyword functionality in define-method rather than also
providing define-method* if one doesn't want to introduce overhead for the
most common types of methods.

Perhaps I could try to learn what is going on in peval and see if a macro
similar to Marks would be the most elegant way to go forward.

You did mention backwards compatibility, but how serious you expect the
> issue would be?  I personally did not use GOOPS yet, but I have a hard
> time imagining a real-world code that would be broken by this change.
> Do you expect there would actually be any?
>

I said "backwards compatibility perspective" and was too lazy to spell out
what I meant, which is this: Assume that we *now* want to write backwards
compatible code. Then, if we have *not* modified define-method we can be
certain that as long as we use define-method, we will be backwards
compatible. However, if we introduce keywords in define-method (rather than
define-method*) this is no longer true. If this new functionality is
confined to define-method*, then a future code can test for the presence of
defined-method* in Guile and otherwise provide something similar to Marks
(or Anders Vinjars from April 2003 :-)) macro.


>
> I personally would probably lean towards two separate procedures (mainly
> due to the assumption of there being a performance impact).
>

Well, there is none now that we have Andys remarkable compiler. Now also
Janneke has responded and he prefers to have everything in define-method.
Myself, I'm split. I can see merits in both solutions. Least code bloat
would probably be to just apply my patch. But I think we should think
mostly about style (where I am still split).

Best regards,
Mikael


>
> Have a nice day,
> Tomas Volf
>
> --
> There are only two hard things in Computer Science:
> cache invalidation, naming things and off-by-one errors.
>

[-- Attachment #2: Type: text/html, Size: 4190 bytes --]

Re: Keywords in GOOPS methods

[janneke]

Mikael Djurfeldt writes:

Hello,

> That was elegant. :-)

:)

> Nope---haven't seen it. (Or, at least I do not remember it.)
>
> Maybe I should have a look at what the optimizer can do about Mark's
> code. (As you might have seen, my code is a modification of the method
> syntax implementation itself.)

Yes, I didn't quite get if this brings something else too, except for
the obvious

> Any opinions on what is best: Having a define-method* or having the
> functionality in define-method itself?

...which I think in your version, not having to use define-method*
feels more elegant/GOOPSy to me, as it's all overloads/generics,
but I have no strong opionion on this.  Thanks for working on this!

Greetings,
Janneke

-- 
Janneke Nieuwenhuizen <janneke@gnu.org>  | GNU LilyPond https://LilyPond.org
Freelance IT https://www.JoyOfSource.com | Avatar® https://AvatarAcademy.com

Re: Keywords in GOOPS methods

[Mikael Djurfeldt]

Thanks!

See responses to Tomas.

On Fri, Nov 22, 2024 at 1:20 PM <janneke@gnu.org> wrote:

> Mikael Djurfeldt writes:
>
> Hello,
>
> > That was elegant. :-)
>
> :)
>
> > Nope---haven't seen it. (Or, at least I do not remember it.)
> >
> > Maybe I should have a look at what the optimizer can do about Mark's
> > code. (As you might have seen, my code is a modification of the method
> > syntax implementation itself.)
>
> Yes, I didn't quite get if this brings something else too, except for
> the obvious
>
> > Any opinions on what is best: Having a define-method* or having the
> > functionality in define-method itself?
>
> ...which I think in your version, not having to use define-method*
> feels more elegant/GOOPSy to me, as it's all overloads/generics,
> but I have no strong opionion on this.  Thanks for working on this!
>
> Greetings,
> Janneke
>
> --
> Janneke Nieuwenhuizen <janneke@gnu.org>  | GNU LilyPond
> https://LilyPond.org
> Freelance IT https://www.JoyOfSource.com | Avatar®
> https://AvatarAcademy.com
>

Re: Keywords in GOOPS methods

[Mikael Djurfeldt]

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

ChatGPT is also split...:

Given the circumstances, I would recommend extending define-method rather
than introducing define-method*.

Here’s why:

   1.

   *Consistency and Simplicity*: By extending define-method, you avoid
   creating a dual system where users have to choose between define-method
   and define-method*. This reduces cognitive overhead and makes the API
   cleaner.
   2.

   *Backwards Compatibility*: As long as the extension maintains backwards
   compatibility—e.g., methods defined without optional, keyword, or rest
   arguments continue to work as before—there’s minimal risk of breaking
   existing code.
   3.

   *Ease of Use*: Users generally prefer fewer forms to remember. Adding
   more features to the existing define-method aligns with the principle of
   least surprise for anyone using GOOPS.
   4.

   *Future-Proofing*: Creating a parallel form like define-method* could
   set a precedent for duplicating functionality every time a significant
   feature is added, leading to API bloat over time.

The counterargument might be that adding optional, keyword, and rest
arguments complicates define-method’s semantics. However, if these features
integrate naturally and don’t break current assumptions, the benefits of a
unified approach outweigh the risks.

My confidence in this recommendation is 85%, as the best decision
ultimately depends on the specifics of your user base and how GOOPS is
typically used. If there's significant risk of confusion or breaking
changes in the extension, introducing define-method* might then become a
better option.
4o
You said:
But Guile already has define* and lambda* forms...
ChatGPT said:

The existence of define* and lambda* in Guile suggests a precedent for
introducing parallel forms to extend functionality. This could justify
introducing define-method* for consistency with Guile's conventions. Here's
a revised analysis based on this:

   1.

   *Consistency with Guile*: Introducing define-method* aligns GOOPS with
   Guile's existing patterns, making it feel more integrated into the Guile
   ecosystem. Users familiar with Guile would likely expect an extended form
   with a * suffix for enhanced functionality.
   2.

   *Avoiding Overloading Semantics*: Extending define-method could make its
   behavior more complex and potentially harder to understand for users who
   only need the basic functionality. Having a separate define-method*
   clearly distinguishes the advanced features, keeping both forms
   conceptually simpler.
   3.

   *Backward Compatibility and Clarity*: A new form eliminates any chance
   of subtle incompatibilities or ambiguities arising from extending
   define-method. For example, users writing macros or tooling around
   define-method won't need to account for new syntax or semantics.
   4.

   *Gradual Adoption*: With define-method*, users can adopt the new
   functionality incrementally. This reduces friction for existing users who
   may not need or want the added complexity of optional, keyword, and rest
   arguments.

Given Guile's * convention, introducing define-method* appears to be the
better option. It respects established practices, avoids over-complicating
existing forms, and provides a clear upgrade path. My confidence in this
recommendation is 90%, given that the Guile precedent is a strong argument
for following suit.
4o

On Fri, Nov 22, 2024 at 1:29 PM Mikael Djurfeldt <mikael@djurfeldt.com>
wrote:

> Thanks!
>
> See responses to Tomas.
>
> On Fri, Nov 22, 2024 at 1:20 PM <janneke@gnu.org> wrote:
>
>> Mikael Djurfeldt writes:
>>
>> Hello,
>>
>> > That was elegant. :-)
>>
>> :)
>>
>> > Nope---haven't seen it. (Or, at least I do not remember it.)
>> >
>> > Maybe I should have a look at what the optimizer can do about Mark's
>> > code. (As you might have seen, my code is a modification of the method
>> > syntax implementation itself.)
>>
>> Yes, I didn't quite get if this brings something else too, except for
>> the obvious
>>
>> > Any opinions on what is best: Having a define-method* or having the
>> > functionality in define-method itself?
>>
>> ...which I think in your version, not having to use define-method*
>> feels more elegant/GOOPSy to me, as it's all overloads/generics,
>> but I have no strong opionion on this.  Thanks for working on this!
>>
>> Greetings,
>> Janneke
>>
>> --
>> Janneke Nieuwenhuizen <janneke@gnu.org>  | GNU LilyPond
>> https://LilyPond.org
>> Freelance IT https://www.JoyOfSource.com | Avatar®
>> https://AvatarAcademy.com
>>
>

[-- Attachment #2: Type: text/html, Size: 14037 bytes --]

Keywords in define-method

[Mikael Djurfeldt]

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

On Fri, Nov 22, 2024 at 1:20 PM <janneke@gnu.org> wrote:

> ...which I think in your version, not having to use define-method*
> feels more elegant/GOOPSy to me, as it's all overloads/generics,
> but I have no strong opionion on this.
>

There's actually a secret to what I have committed:

define-method* *is* the define-method you requested.

So you can do, e.g.:

(use-modules ((oop goops)
              #:hide (define-method)
              #:renamer (lambda (s) (if (eq? s 'define-method*)
'define-method s))))

or

(define-module (foo)
  #:use-module ((oop goops)
                #:hide (define-method)
                #:renamer (lambda (s) (if (eq? s 'define-method*)
'define-method s))))

and then

(define-method (bar #:key x) x)

For ordinary formals, like

(define-method (baz x) ...)

the resulting behavior and efficiency will be very close to identical. The
compiled code and type dispatch of the method will be identical to that
produced by the original define-method macro.

Best regards,
Mikael

[-- Attachment #2: Type: text/html, Size: 2152 bytes --]

Re: Keywords in define-method

[Mikael Djurfeldt]

Den tis 26 nov. 2024 00:51Mikael Djurfeldt <mikael@djurfeldt.com> skrev:

> On Fri, Nov 22, 2024 at 1:20 PM <janneke@gnu.org> wrote:
>
>> ...which I think in your version, not having to use define-method*
>> feels more elegant/GOOPSy to me, as it's all overloads/generics,
>> but I have no strong opionion on this.
>>
>
> There's actually a secret to what I have committed:
>
> define-method* *is* the define-method you requested.
>
> So you can do, e.g.:
>
> (use-modules ((oop goops)
>               #:hide (define-method)
>               #:renamer (lambda (s) (if (eq? s 'define-method*)
> 'define-method s))))
>
> or
>
> (define-module (foo)
>   #:use-module ((oop goops)
>                 #:hide (define-method)
>                 #:renamer (lambda (s) (if (eq? s 'define-method*)
> 'define-method s))))
>
> and then
>
> (define-method (bar #:key x) x)
>
> For ordinary formals, like
>
> (define-method (baz x) ...)
>
> the resulting behavior and efficiency will be very close to identical.
>

That was a bit unclear. What I meant was that for ordinary formals
define-method* compiles almost as fast as define-method and:

The compiled code and type dispatch of the method will be identical to that
> produced by the original define-method macro.
>

Re: Keywords in define-method

[Mikael Djurfeldt]

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

Sorry for all spam, but I realize that R6RS does this better:

(import (rename (except (oop goops) define-method) (define-method*
define-method)))

Maybe we should have a #:rename ((ORIG . SEEN) ...) in the Guile native
module system? Or perhaps we should gradually move over to R6RS?

Best regards,
Mikael

[-- Attachment #2: Type: text/html, Size: 512 bytes --]

Re: Keywords in define-method

[Mikael Djurfeldt]

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

I just committed a module (oop goops keyword-formals) for users who prefer
not to switch between define-method and define-method*.

If you do

  (use-modules (oop goops) (oop goops keyword-formals))

you'll get a define-method which takes keyword formals.

Best regards,
Mikael

[-- Attachment #2: Type: text/html, Size: 469 bytes --]

RE: Keywords in GOOPS methods

[Maxime Devos via General Guile related discussions]

>Any opinions on what is best: Having a define-method* or having the functionality in define-method itself?

You can’t unify define-method with define-method* without making some arbitrary choices in some special cases (the same applies to define-method* too actually, and also to define-method on its own without keyword arguments, but it needs to be considered and documented somewhere).

Consider (please ignore syntax errors, don’t have much practice with GOOPS):

(define-method (f (a <keyword>) (b <integer>))
  (pk 'positional a b))

;; what I mean is to only consider ‘foo’ that are of class <number>,  I don’t mean <number> as default value
(define-method (f (#:key foo <number>)) 
  (pk 'keyword foo))

An ambiguous case: (f #:foo 1). This matches both the first and second implementation. <integer> is more specific that <number>, so this sounds like the first method should win. But, ‘#:foo’ is more specific than <keyword>, so the second should win.

I don’t know what the rule is for positional arguments (I assume earlier arguments are more important in case of ambiguity?). However, the same rule cannot be applied to keyword arguments, since they aren’t positional. Consider:

(define-method (g (#:key foo <integer>) (#:key bar <object>))
  (pk 'integer foo))

(define-method (g (#:key bar <symbol>) (#:key foo <object>))
  (pk 'symbol bar))

(g #:foo 1 #:bar a) ;integer or symbol?
(g #:bar a #:foo 1) ;does this produce the same result?

Either ‘integer’ or ‘symbol’ would be appropriate, but it shouldn’t depend on whether #:foo or #:bar is written first in the argument call. Now #:foo and #:bar need to be (ideally deterministically) ordered, but any particular ordering is somewhat arbitrary, and it’s a bit against the notion of keyword arguments – orderings are more a thing for _positional_ arguments.

Potential escape: in case of ambiguity, give up and throw some kind of exception. Not ideal, but at least it’s not random, doesn’t depend on how the keyword arguments were worded, and it would be limited to keyword methods.

Best regards,
Maime Devos

Re: Keywords in GOOPS methods

[Mikael Djurfeldt]

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

Hi Maxime,

Well, these particular examples aren't valid since GOOPS doesn't allow type
specifiers for keyword arguments. (It's the same in CLOS.) Type dispatch is
done *only* on the required arguments.

Best regards,
Mikael

On Sat, Nov 23, 2024 at 4:31 PM Maxime Devos <maximedevos@telenet.be> wrote:

> >Any opinions on what is best: Having a define-method* or having the
> functionality in define-method itself?
>
>
>
> You can’t unify define-method with define-method* without making some
> arbitrary choices in some special cases (the same applies to define-method*
> too actually, and also to define-method on its own without keyword
> arguments, but it needs to be considered and documented somewhere).
>
>
>
> Consider (please ignore syntax errors, don’t have much practice with
> GOOPS):
>
>
>
> (define-method (f (a <keyword>) (b <integer>))
>   (pk 'positional a b))
>
>
>
> ;; what I mean is to only consider ‘foo’ that are of class <number>,  I
> don’t mean <number> as default value
>
> (define-method (f (#:key foo <number>))
>   (pk 'keyword foo))
>
>
>
> An ambiguous case: (f #:foo 1). This matches both the first and second
> implementation. <integer> is more specific that <number>, so this sounds
> like the first method should win. But, ‘#:foo’ is more specific than
> <keyword>, so the second should win.
>
>
>
> I don’t know what the rule is for positional arguments (I assume earlier
> arguments are more important in case of ambiguity?). However, the same rule
> cannot be applied to keyword arguments, since they aren’t positional.
> Consider:
>
>
>
> (define-method (g (#:key foo <integer>) (#:key bar <object>))
>
>   (pk 'integer foo))
>
>
>
> (define-method (g (#:key bar <symbol>) (#:key foo <object>))
>
>   (pk 'symbol bar))
>
>
>
> (g #:foo 1 #:bar a) ;integer or symbol?
>
> (g #:bar a #:foo 1) ;does this produce the same result?
>
>
>
> Either ‘integer’ or ‘symbol’ would be appropriate, but it shouldn’t depend
> on whether #:foo or #:bar is written first in the argument call. Now #:foo
> and #:bar need to be (ideally deterministically) ordered, but any
> particular ordering is somewhat arbitrary, and it’s a bit against the
> notion of keyword arguments – orderings are more a thing for _*positional*_
> arguments.
>
>
>
> Potential escape: in case of ambiguity, give up and throw some kind of
> exception. Not ideal, but at least it’s not random, doesn’t depend on how
> the keyword arguments were worded, and it would be limited to keyword
> methods.
>
>
>
> Best regards,
>
> Maime Devos
>

[-- Attachment #2: Type: text/html, Size: 5052 bytes --]

RE: Keywords in GOOPS methods

[Maxime Devos via General Guile related discussions]

>Well, these particular examples aren't valid since GOOPS doesn't allow type specifiers for keyword arguments. (It's the same in CLOS.) Type dispatch is done *only* on the required arguments.

That’s a shame, it should support them IMO, it’s a major limitation if it doesn’t.

A variant with untyped keyword arguments:

(define-method (f (a <keyword>) (b <symbol>))
  (pk 'positional))

(define-method (f (#:key foo))
  (pk 'optional-keyword foo))

(f #:foo 'bar)

Who should win? Both are a quite specific match.

Best regards,
Maxime Devos

Re: Keywords in GOOPS methods

[Mikael Djurfeldt]

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

(It's (define-method (f #:key foo) ...).)

This is a valid point. Certainly there should be unambiguous rules so that
we know which method should be selected. (I planned to examine this aspect
before applying my patch to Guile. I was just eager to share what I had
done in order to collect opinions before putting too much effort into it.)

In this case, I would say that positional *should* win since it is most
specific with regards to the specifiers.

The patch I posted actually doesn't result in this, but it could be easily
fixed with a change based on the following consideration:

The motivation for having what you call the "major limitation" of not doing
type dispatch on keyword arguments in GOOPS or CLOS is to strike a
reasonable balance with regards to the complexity of implementation (and
cognitive load for the user, I guess :). In the future, if we come up with
a conceptually simple picture of how to regard method precedence in the
light of typed keyword arguments and also see that this doesn't make the
implementation overly complicated, we can consider extending the
functionality in this direction.

CLOS has a fixed number of required arguments per generic function.  So, in
CLOS it isn't possible to come up with two allowed methods where the order
of dispatch is unclear.

We loosed that up in GOOPS and allow varying number of arguments (which is
a Good Thing). So, you are right that we need to come up with rules that
make the order of dispatch clear.

I propose that the simplest rule which corresponds to restricting type
dispatch to non-keyword arguments is to define the specializer list of
every method with keyword formals like this:

Method formals: ((<F1> <TYPE1>) ... <KEYWORD> ...)

results in

Specializer list: (<TYPE1> ... . <top>)

So

  (method ((x <integer>) . y) ...)

will have the same specializers as

  (method* ((x <integer>) #:key z) ...)

With this definition, positional above will win. In my patch, this
corresponds to changing #''() to #'<top> in the computation of specializers
for keyword methods.

Meanwhile, I've found a further bug in my patch: We need to consider
keyword arguments in re-definition in methods. Fixed now in my still
private code.

Also, I should add that I now lean towards supplying the keyword
functionality in additional syntax method* and define-method*, and keep the
semantics for method and define-method as is.

Best regards,
Mikael

On Sat, Nov 23, 2024 at 4:49 PM Maxime Devos <maximedevos@telenet.be> wrote:

> >Well, these particular examples aren't valid since GOOPS doesn't allow
> type specifiers for keyword arguments. (It's the same in CLOS.) Type
> dispatch is done *only* on the required arguments.
>
>
>
> That’s a shame, it should support them IMO, it’s a major limitation if it
> doesn’t.
>
>
>
> A variant with untyped keyword arguments:
>
>
>
> (define-method (f (a <keyword>) (b <symbol>))
>
>   (pk 'positional))
>
>
>
> (define-method (f (#:key foo))
>
>   (pk 'optional-keyword foo))
>
>
>
> (f #:foo 'bar)
>
>
>
> Who should win? Both are a quite specific match.
>
>
>
> Best regards,
>
> Maxime Devos
>

[-- Attachment #2: Type: text/html, Size: 5294 bytes --]