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

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

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).



Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]>
>
> 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



Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]>

> > 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).



Reply | Threaded
Open this post in threaded view
|

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: [hidden email]
> From: Alan Mackenzie <[hidden email]>
>
> 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.



Reply | Threaded
Open this post in threaded view
|

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

Alan Mackenzie
In reply to this post by 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).



Reply | Threaded
Open this post in threaded view
|

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

Stefan Kangas
In reply to this post by Eli Zaretskii
Eli Zaretskii <[hidden email]> writes:

>> Date: Sat, 28 Sep 2019 12:30:34 +0000
>> From: Alan Mackenzie <[hidden email]>
>>
>> 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



Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]> writes:

> >> Date: Sat, 28 Sep 2019 12:30:34 +0000
> >> From: Alan Mackenzie <[hidden email]>

> >> 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).



Reply | Threaded
Open this post in threaded view
|

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

Alan Mackenzie
In reply to this post by Alan Mackenzie
Wishlist item committed to master.

--
Alan Mackenzie (Nuremberg, Germany).