GNU bug report logs - #6415
23.1.50; edebug-eval-defun errors on dotted pair in some macros

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 6415 in the body.
You can then email your comments to 6415 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Geoff Gole <geoffgole <at> gmail.com>
To: emacs-pretest-bug <at> gnu.org
Subject: 23.1.50; edebug-eval-defun errors on dotted pair in some macros
Date: Sun, 13 Jun 2010 23:26:07 +0800

When presented with a reasonable defun form containing an unevaluated
dotted pair, edebug-eval-defun fails with

  Invalid read syntax: "Dotted spec required."

I *think* that this is an error in the cl.el debug specs and not
edebug itself. Unfortunately that's hard to verify by stepping
edebug.el as said debug specs are largely incomprehensible.

To reproduce:

  emacs -Q
  insert (defun bug () (destructuring-bind (x . y)))
  C-u C-M-x

GNU Emacs 23.1.50.1 (i686-pc-linux-gnu, GTK+ Version 2.12.11) of 2009-07-30

Message #12 received at 6415 <at> debbugs.gnu.org (full text, mbox):

From: Lars Magne Ingebrigtsen <larsi <at> gnus.org>
To: Leo <sdl.web <at> gmail.com>
Cc: 6415 <at> debbugs.gnu.org
Subject: Re: 23.2; error: (invalid-read-syntax "Dotted spec required.")
Date: Wed, 21 Sep 2011 22:34:57 +0200

Leo <sdl.web <at> gmail.com> writes:

> C-u C-M-x to edebug the following example function
>
> (defun test ()
>   (destructuring-bind (beg . end)
>       '(1 . 2)))
>
> Should see a backtrace as attached to the end of this report.
>
> In GNU Emacs 23.2.9 of 2010-06-26 on Victoria.local
>
> Debugger entered--Lisp error: (invalid-read-syntax "Dotted spec required.")

I can confirm that this bug is still present in Emacs 24.

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

Message #15 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Steve Yegge <stevey <at> google.com>
To: bug-gnu-emacs <at> gnu.org
Subject: bug#6415: [PATCH] fix edebug instrumentation of dotted pairs in macros
Date: Mon, 26 Sep 2011 10:17:52 -0700

[Message part 1 (text/plain, inline)]

** Description

There were two separate problems conspiring to create this bug.

First, the edebug spec for `destructuring-bind' is incorrect.
Its definition has three violations of the CL hyperspec for
destructuring lambda lists:

  - it should not support the &environment keyword
  - it should support the &whole keyword
  - it should support dotted forms

It so happens that the `cl-macro-list1' edebug-spec does all three
of these things properly.

The second problem is in edebug.  The unification algorithm has
improper or missing handling for dotted pairs in specs.  I chose
to add the handling to `edebug-match-specs' since it seemed to be
the cleanest place to insert it.

** ChangeLog

2011-09-26  Steve Yegge  <stevey <at> google.com>

* emacs-lisp/cl-specs.el: Fixed edebug-spec for
`destructuring-bind' to allow dotted pairs in the
destructuring lambda list.  (Bug#6415)

* emacs-lisp/edebug.el: Fixed edebug instrumentation of
dotted pairs in edebug specifications for macros.  (Bug#6415)

** The patch itself

=== modified file 'lisp/emacs-lisp/cl-specs.el'
--- lisp/emacs-lisp/cl-specs.el 2011-02-11 03:54:12 +0000
+++ lisp/emacs-lisp/cl-specs.el 2011-09-26 16:37:19 +0000
@@ -90,7 +90,7 @@
   ((&rest (symbol sexp)) cl-declarations body))

 (def-edebug-spec destructuring-bind
-  (&define cl-macro-list def-form cl-declarations def-body))
+  (&define cl-macro-list1 def-form cl-declarations def-body))

 ;; Setf


=== modified file 'lisp/emacs-lisp/edebug.el'
--- lisp/emacs-lisp/edebug.el 2011-08-21 17:43:31 +0000
+++ lisp/emacs-lisp/edebug.el 2011-09-26 16:44:39 +0000
@@ -1567,8 +1567,28 @@
       (let ((edebug-dotted-spec t));; Containing spec list was dotted.
  (edebug-match-specs cursor (list specs) remainder-handler)))

-     ;; Is the form dotted?
-     ((not (listp (edebug-cursor-expressions cursor)));; allow nil
+     ;; Special handling for the tail of a dotted form.
+     ((and
+       ;; Is the cursor on the tail of a dotted form?
+       (not (listp (edebug-cursor-expressions cursor)));; allow nil
+       ;; When matching a dotted form such as (a b . c) against a
+       ;; spec list that looks like
+       ;;     ([&rest ...] [&optional ...]+ . [&or arg nil])
+       ;; ,e.g., the `cl-macro-list1' edebug-spec, then the &rest spec
+       ;; will consume everything up to the dotted tail (`c' in this
+       ;; example).  At that point the spec list will look like so:
+       ;;     ([&optional ...]+ . [&or arg nil])
+       ;; We need to be able to consume zero or more [&optional ...]
+       ;; spec(s) without moving the cursor or signaling an error.
+       ;; The current continuation provides no state that tells us
+       ;; about the upcoming &optional specs, so we use lookahead:
+
+       ;; Recurse normally if we're about to process an optional spec.
+       (not (eq (car specs) '&optional))
+       ;; Recurse normally if the spec is a dotted list.
+       (not (and (listp specs)
+                 (not (listp (cdr (last specs)))))))
+      ;; Otherwise we need to be on the tail of a dotted spec.
       (if (not edebug-dotted-spec)
   (edebug-no-match cursor "Dotted spec required."))
       ;; Cancel dotted spec and dotted form.

[Message part 2 (text/html, inline)]

Message #18 received at 6415 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Steve Yegge <stevey <at> google.com>
Cc: 6415 <at> debbugs.gnu.org
Subject: Re: bug#6415: [PATCH] fix edebug instrumentation of dotted pairs in
	macros
Date: Mon, 26 Sep 2011 21:43:46 -0400

> It so happens that the `cl-macro-list1' edebug-spec does all three
> of these things properly.

I haven't looked into it, so I'll trust on that one.

> The second problem is in edebug.  The unification algorithm has
> improper or missing handling for dotted pairs in specs.  I chose
> to add the handling to `edebug-match-specs' since it seemed to be
> the cleanest place to insert it.

This edebug-dotted-spec business is really ugly, I wonder if/how we
could just get rid of this variable.  Or at least document clearly what
it is supposed to mean.

> -     ;; Is the form dotted?
> -     ((not (listp (edebug-cursor-expressions cursor)));; allow nil
> +     ;; Special handling for the tail of a dotted form.
> +     ((and
> +       ;; Is the cursor on the tail of a dotted form?
> +       (not (listp (edebug-cursor-expressions cursor)));; allow nil
> +       ;; When matching a dotted form such as (a b . c) against a
> +       ;; spec list that looks like
> +       ;;     ([&rest ...] [&optional ...]+ . [&or arg nil])
> +       ;; ,e.g., the `cl-macro-list1' edebug-spec, then the &rest spec
> +       ;; will consume everything up to the dotted tail (`c' in this
> +       ;; example).  At that point the spec list will look like so:
> +       ;;     ([&optional ...]+ . [&or arg nil])
> +       ;; We need to be able to consume zero or more [&optional ...]
> +       ;; spec(s) without moving the cursor or signaling an error.
> +       ;; The current continuation provides no state that tells us
> +       ;; about the upcoming &optional specs, so we use lookahead:
> +
> +       ;; Recurse normally if we're about to process an optional spec.
> +       (not (eq (car specs) '&optional))
> +       ;; Recurse normally if the spec is a dotted list.
> +       (not (and (listp specs)
> +                 (not (listp (cdr (last specs)))))))
> +      ;; Otherwise we need to be on the tail of a dotted spec.
>        (if (not edebug-dotted-spec)
>    (edebug-no-match cursor "Dotted spec required."))
>        ;; Cancel dotted spec and dotted form.

Questions:
- Should it really only be &optional?  it looks like any &foo might work
  just as well.  Also shouldn't we check the (eq (aref (car specs) 0)
  '&optional) instead?
- What's the purpose of the
  (not (and (listp specs) (not (listp (cdr (last specs))))))?
  For one (listp specs) will always be t (we've checked (atom specs)
  earlier and we've just called (car specs) on the previous line)
  so the code is really (not (and t (not (listp (cdr (last specs))))))
  aka (listp (cdr (last specs))) but if the car of specs is not an
  &optional, then we have a mismatch anyway, no?


        Stefan

Message #25 received at 6415 <at> debbugs.gnu.org (full text, mbox):

From: Gemini Lasswell <gazally <at> runbox.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 6415 <at> debbugs.gnu.org, Steve Yegge <stevey <at> google.com>
Subject: Re: bug#6415: [PATCH] fix edebug instrumentation of dotted pairs in
 macros
Date: Sun, 05 Nov 2017 13:45:21 -0800

[Message part 1 (text/plain, inline)]

Here's a new patch for this bug, based on the ideas in Steve's patch.
It also allows &rest and specs wrapped in vectors to attempt to match
before a dotted tail.

[0001-Fix-Edebug-s-handling-of-dotted-specs-bug-6415.patch (text/plain, attachment)]

[Message part 3 (text/plain, inline)]

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> This edebug-dotted-spec business is really ugly, I wonder if/how we
> could just get rid of this variable.  Or at least document clearly what
> it is supposed to mean.

I agree. After far too many hours of looking at this, I think the way
to get rid of the variable is to make Edebug's internal representation
of its specs into a cl-defstruct so there is room for a "dotted" flag,
and then change all the edebug match code so that whenever it makes a
new spec or modifies the one it is working with, it inherits the
dotted flag. I have a branch with this partially done and am confident
that it will work and not cause a performance problem, but it seems
like a lot of work and a lot of changes to stable code to make it work
exactly the same. So I added a docstring.

Message #30 received at 6415-done <at> debbugs.gnu.org (full text, mbox):

From: Gemini Lasswell <gazally <at> runbox.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 6415-done <at> debbugs.gnu.org, Steve Yegge <stevey <at> google.com>
Subject: Re: bug#6415: [PATCH] fix edebug instrumentation of dotted pairs in
 macros
Date: Sun, 26 Nov 2017 15:02:27 -0800

Gemini Lasswell <gazally <at> runbox.com> writes:

> Here's a new patch for this bug, based on the ideas in Steve's patch.

I've pushed this patch to emacs-26.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.