bug#30910: 26.0.91; Incorrect Edebug spec for def-edebug-spec

bug#30910: 26.0.91; Incorrect Edebug spec for def-edebug-spec

[Gemini Lasswell]

def-edebug-spec has an Edebug spec, which fails to match the valid
Edebug specs in lisp/emacs-lisp/cl-macs.el.

To reproduce, load cl-macs.el and:

M-x edebug-all-defs RET
M-x eval-buffer RET

Result: edebug-syntax-error: Invalid read syntax: "Expected one of",
"nil", edebug-spec-p, "t", "0", (&rest edebug-spec)

A simpler edebug spec than cl-lambda-list which also causes the error
is:

(def-edebug-spec buggy ((symbolp symbolp symbolp symbolp . [&or symbolp nil])))

Fewer than four symbolp's will not produce an error.

bug#30910: 26.0.91; Incorrect Edebug spec for def-edebug-spec

[Lars Ingebrigtsen]

Gemini Lasswell <gazally@runbox.com> writes:

> def-edebug-spec has an Edebug spec, which fails to match the valid
> Edebug specs in lisp/emacs-lisp/cl-macs.el.
>
> To reproduce, load cl-macs.el and:
>
> M-x edebug-all-defs RET
> M-x eval-buffer RET
>
> Result: edebug-syntax-error: Invalid read syntax: "Expected one of",
> "nil", edebug-spec-p, "t", "0", (&rest edebug-spec)

This bug is still present on the trunk.

But you say that it has an edebug spec, but:

(symbol-plist 'def-edebug-spec)
=> nil

I thought that perhaps saying something like

(eval-and-compile
  (put 'def-edebug-spec 'edebug-form-spec '(sexp sexp)))

might be a fix, but it doesn't seem to have any effect...

Here's the backtrace:

Debugger entered--Lisp error: (invalid-read-syntax "Expected one of" "nil" ede
  signal(invalid-read-syntax ("Expected one of" "nil" edebug-spec-p "t" "0" (&
  edebug-syntax-error("Expected one of" "nil" edebug-spec-p "t" "0" (&rest ede
  apply(edebug-syntax-error ("Expected one of" "nil" edebug-spec-p "t" "0" (&r
  edebug-no-match((((([&rest cl-lambda-arg] [&optional ["&optional" cl-&option
  apply(edebug-no-match (((([&rest cl-lambda-arg] [&optional ["&optional" cl-&
  edebug-match-&or((((([&rest cl-lambda-arg] [&optional ["&optional" cl-&optio
  edebug-match-specs((((([&rest cl-lambda-arg] [&optional ["&optional" cl-&opt
  edebug-match-specs((((([&rest cl-lambda-arg] [&optional ["&optional" cl-&opt
  edebug-match-specs((((([&rest cl-lambda-arg] [&optional ["&optional" cl-&opt
  edebug-match-specs((((([&rest cl-lambda-arg] [&optional ["&optional" cl-&opt
  edebug-match-sublist((((([&rest cl-lambda-arg] [&optional ["&optional" cl-&o
  edebug-match-list((((def-edebug-spec cl-lambda-list (([&rest cl-lambda-arg] 
  edebug-match-one-spec((((def-edebug-spec cl-lambda-list (([&rest cl-lambda-a
  edebug-match-specs((((def-edebug-spec cl-lambda-list (([&rest cl-lambda-arg]
  edebug-match((((def-edebug-spec cl-lambda-list (([&rest cl-lambda-arg] [&opt
  edebug-make-form-wrapper((((def-edebug-spec cl-lambda-list (([&rest cl-lambd
  edebug-read-and-maybe-wrap-form1()
  edebug-read-and-maybe-wrap-form()

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#30910: 26.0.91; Incorrect Edebug spec for def-edebug-spec

[Gemini Lasswell]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> But you say that it has an edebug spec, but:
>
> (symbol-plist 'def-edebug-spec)
> => nil

Do (require 'edebug) first, then that will work.

> I thought that perhaps saying something like
>
> (eval-and-compile
>   (put 'def-edebug-spec 'edebug-form-spec '(sexp sexp)))
>
> might be a fix, but it doesn't seem to have any effect...

(def-edebug-spec def-edebug-spec (&rest sexp))

would be a fix, but so would be deleting it entirely.

Since there is no code needing to be debugged inside of an Edebug spec,
the only reason I can think of for having an Edebug spec for
def-edebug-spec is to check whether the specs defined with it are
validly formed.  The error messages issued during Edebug spec matching
are not very good, but maybe someone will be inspired to do something
about that someday.

Right now the only Edebug specs that get matched to the def-edebug-spec
definition are the ones declared with def-edebug-spec, not the ones in
macro definitions, because:

(get-edebug-spec 'defmacro)
=> (&define name lambda-list lambda-doc
	 [&optional
	  ("declare" &rest sexp)]
	 def-body)

If we had a Edebug spec for Edebug specs that wasn't broken, we could
change the defmacro spec to use it, and see what happens.

bug#30910: 26.0.91; Incorrect Edebug spec for def-edebug-spec

[Lars Ingebrigtsen]

Gemini Lasswell <gazally@runbox.com> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> But you say that it has an edebug spec, but:
>>
>> (symbol-plist 'def-edebug-spec)
>> => nil
>
> Do (require 'edebug) first, then that will work.

Ah, right:

(symbol-plist 'def-edebug-spec)
=> (edebug-form-spec (&define :name edebug-spec name &or "nil" edebug-spec-p "t" "0" (&rest edebug-spec)))

>> I thought that perhaps saying something like
>>
>> (eval-and-compile
>>   (put 'def-edebug-spec 'edebug-form-spec '(sexp sexp)))
>>
>> might be a fix, but it doesn't seem to have any effect...
>
> (def-edebug-spec def-edebug-spec (&rest sexp))
>
> would be a fix, but so would be deleting it entirely.

Yeah, I guess.

> Since there is no code needing to be debugged inside of an Edebug spec,
> the only reason I can think of for having an Edebug spec for
> def-edebug-spec is to check whether the specs defined with it are
> validly formed.  The error messages issued during Edebug spec matching
> are not very good, but maybe someone will be inspired to do something
> about that someday.

The current spec was added with this helpful commit message:

commit 1fe3d50701adcd8929745edf24158a4a50459ea0
Author: Daniel LaLiberte <liberte@gnu.org>
Date:   Thu Mar 24 20:38:34 1994 +0000

    New version from author.

And this is the code, that has gone virtually unchanged since 1994.
Note the "Out of date" comment, which was there in 1994:

;;;;* Spec for def-edebug-spec
;;; Out of date.

(defun edebug-spec-p (object)
  "Return non-nil if OBJECT is a symbol with an edebug-form-spec property."
  (and (symbolp object)
       (get object 'edebug-form-spec)))

(def-edebug-spec def-edebug-spec
  ;; Top level is different from lower levels.
  (&define :name edebug-spec name
	   &or "nil" edebug-spec-p "t" "0" (&rest edebug-spec)))

(def-edebug-spec edebug-spec-list
  ;; A list must have something in it, or it is nil, a symbolp
  ((edebug-spec . [&or nil edebug-spec])))

(def-edebug-spec edebug-spec
  (&or
   (vector &rest edebug-spec)		; matches a vector
   ("vector" &rest edebug-spec)		; matches a vector spec
   ("quote" symbolp)
   edebug-spec-list
   stringp
   [edebug-lambda-list-keywordp &rest edebug-spec]
   [keywordp gate edebug-spec]
   edebug-spec-p  ;; Including all the special ones e.g. form.
   symbolp;; a predicate
   ))

So it does sound more like a test for whether the specs are well-formed,
and not really...  anything with edebug proper?

> Right now the only Edebug specs that get matched to the def-edebug-spec
> definition are the ones declared with def-edebug-spec, not the ones in
> macro definitions, because:
>
> (get-edebug-spec 'defmacro)
> => (&define name lambda-list lambda-doc
> 	 [&optional
> 	  ("declare" &rest sexp)]
> 	 def-body)
>
> If we had a Edebug spec for Edebug specs that wasn't broken, we could
> change the defmacro spec to use it, and see what happens.

Right.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no