bug#37540: [PATCH] Wishlist: Allow edebug-specs to signal arbitrary error strings on syntax errors in macros.

bug#37540: [PATCH] Wishlist: Allow edebug-specs to signal arbitrary error strings on syntax errors in macros.

[Alan Mackenzie]

Hello, Emacs.

I think it would be a good idea to allow edebug-specs to signal syntax
errors with arbitrary strings, like:

     ("`" &error "Too deeply nested backquotes")

.  The motivation is to complete the correction in bug #31090, which
fixed edebug's handling of nested backquotes in macros.

The problem with that fix is it doesn't handle triply nested backquotes
(without , or ,@ between them) at all well.  There is a fundamental
structure clash between backquote's nesting and Lisp's nesting of
parentheses.

The pragmatic solution is simply to ban triply nested backquotes (i.e.
three `s without ,s or ,@s between them) from edebug instrumentation.
However, this really needs a mechanism to output an error message
string.  There is currently no such mecahanism in edebug.

The following patch implements an &error mechanism in edebug.el, and
illustrates its use in the case which motivated it.

Any objections to committing this patch (together with the needed
amendments to documentation and NEWS)?




diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 717026995a..73257f1568 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -1683,6 +1683,7 @@ edebug-match-specs
                 (cl-macrolet-body . edebug-match-cl-macrolet-body)
 		(&not . edebug-match-&not)
 		(&key . edebug-match-&key)
+		(&error . edebug-match-&error)
 		(place . edebug-match-place)
 		(gate . edebug-match-gate)
 		;;   (nil . edebug-match-nil)  not this one - special case it.
@@ -1816,6 +1817,14 @@ edebug-match-&key
                            (car (cdr pair))))
 		 specs))))
 
+(defun edebug-match-&error (cursor specs)
+  ;; Signal an error, using the following string in the spec as argument.
+  (setq edebug-gate t)
+  (let* ((error-string (car specs)))
+    (if (stringp error-string)
+        (edebug-no-match cursor error-string)
+      (error "String expected after &error in edebug-spec"))))
+
 
 (defun edebug-match-gate (_cursor)
   ;; Simply set the gate to prevent backtracking at this level.
@@ -2185,6 +2194,8 @@ backquote-form
 
 (def-edebug-spec nested-backquote-form
   (&or
+   ("`" &error "Triply nested backquotes (without commas \"between\" them) \
+are too difficult to instrument")
    ;; Allow instrumentation of any , or ,@ contained within the (\, ...) or
    ;; (\,@ ...) matched on the next line.
    ([&or "," ",@"] backquote-form)


-- 
Alan Mackenzie (Nuremberg, Germany).

bug#37540: [PATCH] Wishlist: Allow edebug-specs to signal arbitrary error strings on syntax errors in macros.

[Eli Zaretskii]

> Date: Sat, 28 Sep 2019 12:30:34 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
> The following patch implements an &error mechanism in edebug.el, and
> illustrates its use in the case which motivated it.
> 
> Any objections to committing this patch (together with the needed
> amendments to documentation and NEWS)?

Please wait with such changes until the emacs-27 branch is cut (which
should happen soon, I hope).  I don't want to risk destabilizing Emacs
27 with potentially risky low-level changes, more than it already is.

TIA

bug#37540: [PATCH] Wishlist: Allow edebug-specs to signal arbitrary error strings on syntax errors in macros.

[Alan Mackenzie]

Hello, Eli.

On Sat, Sep 28, 2019 at 16:42:47 +0300, Eli Zaretskii wrote:
> > Date: Sat, 28 Sep 2019 12:30:34 +0000
> > From: Alan Mackenzie <acm@muc.de>

> > The following patch implements an &error mechanism in edebug.el, and
> > illustrates its use in the case which motivated it.

> > Any objections to committing this patch (together with the needed
> > amendments to documentation and NEWS)?

> Please wait with such changes until the emacs-27 branch is cut (which
> should happen soon, I hope).  I don't want to risk destabilizing Emacs
> 27 with potentially risky low-level changes, more than it already is.

OK.  Presumably when the emacs-27 branch comes into existence, the
emacs-26 branch will be retired from active service.

> TIA

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#37540: [PATCH] Wishlist: Allow edebug-specs to signal arbitrary error strings on syntax errors in macros.

[Eli Zaretskii]

> Date: Sat, 28 Sep 2019 20:34:16 +0000
> Cc: 37540@debbugs.gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> Presumably when the emacs-27 branch comes into existence, the
> emacs-26 branch will be retired from active service.

It is actually de-facto retired already.  Making changes there that
are anything but emergency fixes just means more work for Glenn and
others, who do the merging, and has no useful purpose.

bug#37540: Bug 37540: [PATCH] Wishlist: Allow edebug-specs to signal arbitrary error strings on syntax errors in macros.

[Alan Mackenzie]

Hello Emacs and Eli.

On Sat, Sep 28, 2019 at 12:30:34 +0000, Alan Mackenzie wrote:
> I think it would be a good idea to allow edebug-specs to signal syntax
> errors with arbitrary strings, like:

>      ("`" &error "Too deeply nested backquotes")

> .  The motivation is to complete the correction in bug #31090, which
> fixed edebug's handling of nested backquotes in macros.

> The problem with that fix is it doesn't handle triply nested backquotes
> (without , or ,@ between them) at all well.  There is a fundamental
> structure clash between backquote's nesting and Lisp's nesting of
> parentheses.

> The pragmatic solution is simply to ban triply nested backquotes (i.e.
> three `s without ,s or ,@s between them) from edebug instrumentation.
> However, this really needs a mechanism to output an error message
> string.  There is currently no such mecahanism in edebug.

> The following patch implements an &error mechanism in edebug.el, and
> illustrates its use in the case which motivated it.

> Any objections to committing this patch (together with the needed
> amendments to documentation and NEWS)?

, to which Eli's response was to request me to wait until the emacs-27
release branch had been cut.  That has now happened.

So, are there any objections to me now committing this patch (current
version below, including a documentation amendment), plus a NEWS item,
to the master branch?



diff --git a/doc/lispref/edebug.texi b/doc/lispref/edebug.texi
index 8be8307c75..cfef5c12d1 100644
--- a/doc/lispref/edebug.texi
+++ b/doc/lispref/edebug.texi
@@ -1362,6 +1362,11 @@ Specification List
 is primarily used to generate more specific syntax error messages.  See
 @ref{Backtracking}, for more details.  Also see the @code{let} example.
 
+@item &error
+@code{&error} should be followed by a string, an error message, in the
+edebug-spec; it aborts the instrumentation, displaying the message in
+the minibuffer.
+
 @item @var{other-symbol}
 @cindex indirect specifications
 Any other symbol in a specification list may be a predicate or an
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index b8d2fb5beb..a57c48f1f9 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -1714,6 +1714,7 @@ edebug-match-specs
                 (cl-macrolet-body . edebug-match-cl-macrolet-body)
 		(&not . edebug-match-&not)
 		(&key . edebug-match-&key)
+		(&error . edebug-match-&error)
 		(place . edebug-match-place)
 		(gate . edebug-match-gate)
 		;;   (nil . edebug-match-nil)  not this one - special case it.
@@ -1847,6 +1848,14 @@ edebug-match-&key
                            (car (cdr pair))))
 		 specs))))
 
+(defun edebug-match-&error (cursor specs)
+  ;; Signal an error, using the following string in the spec as argument.
+  (setq edebug-gate t)
+  (let* ((error-string (car specs)))
+    (if (stringp error-string)
+        (edebug-no-match cursor error-string)
+      (error "String expected after &error in edebug-spec"))))
+
 
 (defun edebug-match-gate (_cursor)
   ;; Simply set the gate to prevent backtracking at this level.
@@ -2216,6 +2225,8 @@ backquote-form
 
 (def-edebug-spec nested-backquote-form
   (&or
+   ("`" &error "Triply nested backquotes (without commas \"between\" them) \
+are too difficult to instrument")
    ;; Allow instrumentation of any , or ,@ contained within the (\, ...) or
    ;; (\,@ ...) matched on the next line.
    ([&or "," ",@"] backquote-form)


-- 
Alan Mackenzie (Nuremberg, Germany).

bug#37540: [PATCH] Wishlist: Allow edebug-specs to signal arbitrary error strings on syntax errors in macros.

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 28 Sep 2019 12:30:34 +0000
>> From: Alan Mackenzie <acm@muc.de>
>> 
>> The following patch implements an &error mechanism in edebug.el, and
>> illustrates its use in the case which motivated it.
>> 
>> Any objections to committing this patch (together with the needed
>> amendments to documentation and NEWS)?
>
> Please wait with such changes until the emacs-27 branch is cut (which
> should happen soon, I hope).  I don't want to risk destabilizing Emacs
> 27 with potentially risky low-level changes, more than it already is.

Now that the emacs-27 branch is cut, perhaps it is time to look into
this again?

Best regards,
Stefan Kangas

bug#37540: [PATCH] Wishlist: Allow edebug-specs to signal arbitrary error strings on syntax errors in macros.

[Alan Mackenzie]

Hello, Stefan.

On Thu, Jan 16, 2020 at 15:47:44 +0100, Stefan Kangas wrote:
> Eli Zaretskii <eliz@gnu.org> writes:

> >> Date: Sat, 28 Sep 2019 12:30:34 +0000
> >> From: Alan Mackenzie <acm@muc.de>

> >> The following patch implements an &error mechanism in edebug.el, and
> >> illustrates its use in the case which motivated it.

> >> Any objections to committing this patch (together with the needed
> >> amendments to documentation and NEWS)?

> > Please wait with such changes until the emacs-27 branch is cut (which
> > should happen soon, I hope).  I don't want to risk destabilizing Emacs
> > 27 with potentially risky low-level changes, more than it already is.

> Now that the emacs-27 branch is cut, perhaps it is time to look into
> this again?

Indeed, I posted a post on this bug just on Tuesday.  It has not yet
attracted much feedback.  I suppose it is a particularly arcane topic,
but such an &error feature would enable me to tidy up the edebug-spec for
backquote properly.

> Best regards,
> Stefan Kangas

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#37540: Bug 37540: [PATCH] Wishlist: Allow edebug-specs to signal arbitrary error strings on syntax errors in macros.

[Alan Mackenzie]

Wishlist item committed to master.

-- 
Alan Mackenzie (Nuremberg, Germany).