bug#62256: 28.2; Invalid edebug spec gv-define-simple-setter

bug#62256: 28.2; Invalid edebug spec gv-define-simple-setter

[Matúš Goljer]

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

Insert

(gv-define-simple-setter aref aset)

in a buffer and try to edebug-eval it (C-u C-M-x).

It gives an error which I don't understand, because the second argument
`aset' should match symbolp...

I also tried this on the build from master and the error is the same.

Debugger entered--Lisp error: (invalid-read-syntax "Expected" (&or symbolp lambda-expr))
  signal(invalid-read-syntax ("Expected" (&or symbolp lambda-expr)))
  edebug-syntax-error("Expected" (&or symbolp lambda-expr))
  apply(edebug-syntax-error ("Expected" (&or symbolp lambda-expr)))
  edebug-no-match(((aset) (176 . 180) . 181) "Expected" (&or symbolp lambda-expr))
  edebug-match-list(((aset) (176 . 180) . 181) (&or symbolp lambda-expr))
  edebug-match-one-spec(((aset) (176 . 180) . 181) (&or symbolp lambda-expr))
  edebug-match-specs(((aset) (176 . 180) . 181) ((&or symbolp lambda-expr) &optional sexp) edebug-match-specs)
  edebug-match-specs(((aset) (176 . 180) . 181) (sexp (&or symbolp lambda-expr) &optional sexp) edebug-match-specs)
  edebug-match-sublist(((aset) (176 . 180) . 181) (sexp (&or symbolp lambda-expr) &optional sexp))
  #f(compiled-function (head cursor) #<bytecode 0x194f427789f7f1ec>)(gv-define-simple-setter ((aset) (176 . 180) . 181))
  edebug-list-form(((aset) (176 . 180) . 181))
  edebug-form((((gv-define-simple-setter aref aset)) (146 (147 . 170) (171 . 175) (176 . 180) . 181)))
  edebug-make-form-wrapper((((gv-define-simple-setter aref aset)) (146 (147 . 170) (171 . 175) (176 . 180) . 181)) 146 181 nil)
  edebug-read-and-maybe-wrap-form1()
  edebug-read-and-maybe-wrap-form()
  edebug--read(read #<buffer *scratch*>)
  apply(edebug--read read #<buffer *scratch*>)

-- 
Best regards,
  Matúš Goljer

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

bug#62256: 28.2; Invalid edebug spec gv-define-simple-setter

[Matúš Goljer]

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

Actually, I think the problem is using the spec

(&or symbolp lambda-expr)

instead of

[&or symbolp lambda-expr]

Because the list construct here always makes "a list with content".  So
this spec says "a list with one thing in it, either a symbol or a
lambda" which would match

(gv-define-simple-setter aref (aset))
-- 
Best regards,
  Matúš Goljer

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

bug#62256: 28.2; Invalid edebug spec gv-define-simple-setter

[Eli Zaretskii]

> From: Matúš Goljer <matus.goljer@gmail.com>
> Date: Sat, 18 Mar 2023 14:36:53 +0100
> 
> Actually, I think the problem is using the spec
> 
> (&or symbolp lambda-expr)
> 
> instead of
> 
> [&or symbolp lambda-expr]
> 
> Because the list construct here always makes "a list with content".  So
> this spec says "a list with one thing in it, either a symbol or a
> lambda" which would match
> 
> (gv-define-simple-setter aref (aset))

Stefan, any comments?

bug#62256: 28.2; Invalid edebug spec gv-define-simple-setter

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> From: Matúš Goljer <matus.goljer@gmail.com>
>> Date: Sat, 18 Mar 2023 14:36:53 +0100
>> 
>> Actually, I think the problem is using the spec
>> 
>> (&or symbolp lambda-expr)
>> 
>> instead of
>> 
>> [&or symbolp lambda-expr]
>> 
>> Because the list construct here always makes "a list with content".  So
>> this spec says "a list with one thing in it, either a symbol or a
>> lambda" which would match
>> 
>> (gv-define-simple-setter aref (aset))
>
> Stefan, any comments?

He's right!


        Stefan

bug#62256: 28.2; Invalid edebug spec gv-define-simple-setter

[Matus Goljer]

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

Do you want me to prepare a patch?  I think there's more incorrect specs in
the gv package now.

m.

On Thu, 23 Mar 2023 at 19:07, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> >> From: Matúš Goljer <matus.goljer@gmail.com>
> >> Date: Sat, 18 Mar 2023 14:36:53 +0100
> >>
> >> Actually, I think the problem is using the spec
> >>
> >> (&or symbolp lambda-expr)
> >>
> >> instead of
> >>
> >> [&or symbolp lambda-expr]
> >>
> >> Because the list construct here always makes "a list with content".  So
> >> this spec says "a list with one thing in it, either a symbol or a
> >> lambda" which would match
> >>
> >> (gv-define-simple-setter aref (aset))
> >
> > Stefan, any comments?
>
> He's right!
>
>
>         Stefan
>
>

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

bug#62256: 28.2; Invalid edebug spec gv-define-simple-setter

[Eli Zaretskii]

> From: Matus Goljer <matus.goljer@gmail.com>
> Date: Thu, 23 Mar 2023 19:11:01 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, 62256@debbugs.gnu.org
> 
> Do you want me to prepare a patch?  I think there's more incorrect specs in the gv package now.

Feel free to submit patches for additional specs as well.

Thanks.

bug#62256: 28.2; Invalid edebug spec gv-define-simple-setter

[Matúš Goljer]

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

> Feel free to submit patches for additional specs as well.
>
> Thanks.

I was thinking about the form in `(get 'compiler-macro
'edebug-declaration-spec)` but actually that one is correct.  It's a bit
confusing that top level is always wrapped in () even though that list
wrapper is ignored.

So in the end there's only this one fix.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Fix edebug spec for gv-define-simple-setter --]
[-- Type: text/x-diff, Size: 950 bytes --]

From f99a7d4423008768d7f0a333fb72fc9cabf446ea Mon Sep 17 00:00:00 2001
From: Matus Goljer <matus.goljer@gmail.com>
Date: Sun, 26 Mar 2023 13:20:19 +0200
Subject: [PATCH] Fix edebug spec for gv-define-simple-setter

---
 lisp/emacs-lisp/gv.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index dad91e92a45..ac001af06bd 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -271,7 +271,7 @@ instead the assignment is turned into something equivalent to
     (SETTER ARGS... temp)
     temp)
 so as to preserve the semantics of `setf'."
-  (declare (debug (sexp (&or symbolp lambda-expr) &optional sexp)))
+  (declare (debug (sexp [&or symbolp lambda-expr] &optional sexp)))
   (when (eq 'lambda (car-safe setter))
     (message "Use `gv-define-setter' or name %s's setter function" name))
   `(gv-define-setter ,name (val &rest args)
-- 
2.34.1


[-- Attachment #1.3: Type: text/plain, Size: 40 bytes --]


-- 
Best regards,
  Matúš Goljer

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

bug#62256: 28.2; Invalid edebug spec gv-define-simple-setter

[Eli Zaretskii]

> From: Matúš Goljer <matus.goljer@gmail.com>
> Cc: monnier@iro.umontreal.ca, 62256@debbugs.gnu.org
> Date: Sun, 26 Mar 2023 13:28:06 +0200
> 
> I was thinking about the form in `(get 'compiler-macro
> 'edebug-declaration-spec)` but actually that one is correct.  It's a bit
> confusing that top level is always wrapped in () even though that list
> wrapper is ignored.
> 
> So in the end there's only this one fix.

Thanks, installed on the emacs-29 branch, and closing the bug.

bug#62256: 28.2; Invalid edebug spec gv-define-simple-setter

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> It's a bit confusing that top level is always wrapped in () even
> though that list wrapper is ignored.

Very much agreed, tho it's usually not completely wrong, since it
describes a *list* of arguments.


        Stefan