bug#74018: 31.0.50; Issue with define-peg-ruleset?

bug#74018: 31.0.50; Issue with define-peg-ruleset?

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

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

Package: Emacs
Version: 31.0.50


Augusto pointed out a bug in `peg.el` when you have code like:

    (define-peg-ruleset myrules
      (sign  () (or "+" "-" ""))
      (digit () [0-9])
      (nat   () digit (* digit))
      (int   () sign digit (* digit))
      (float () int "." nat))

    (with-peg-rules
        (myrules
         (complex float "+i" float))
      (peg-parse nat "," nat "," complex) )

where the macroexpanded code is wrong and leads to errors like:

    Debugger entered--Lisp error: (void-function peg-rule\ nat)

I was about to install the bugfix below to `master` but noticed that it
probably belongs to `emacs-30` instead since the bug makes
`define-peg-ruleset` basically unusable and PEG is new in Emacs-30.

Any objection?


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: peg.patch --]
[-- Type: text/x-diff, Size: 1485 bytes --]

diff --git a/lisp/progmodes/peg.el b/lisp/progmodes/peg.el
index 96334162195..b18d68c69c5 100644
--- a/lisp/progmodes/peg.el
+++ b/lisp/progmodes/peg.el
@@ -412,6 +412,7 @@ define-peg-ruleset
              (full-rname (format "%s %s" name rname)))
         (push `(define-peg-rule ,full-rname . ,(cdr rule)) defs)
         (push `(,(peg--rule-id rname) #',(peg--rule-id full-rname)) aliases)))
+    (require 'cl-lib)
     `(cl-flet ,aliases
        ,@defs
        (eval-and-compile (put ',name 'peg--rules ',aliases)))))
@@ -432,7 +432,8 @@
                               (progn (push rule rulesets) nil)
                             (cons (car rule) (peg-normalize `(and . ,(cdr rule))))))
                         rules)))
-        (ctx (assq :peg-rules macroexpand-all-environment)))
+         (ctx (assq :peg-rules macroexpand-all-environment))
+         (body
     (macroexpand-all
      `(cl-labels
           ,(mapcar (lambda (rule)
@@ -444,6 +445,15 @@
         ,@body)
      `((:peg-rules ,@(append rules (cdr ctx)))
        ,@macroexpand-all-environment))))
+    (if (null rulesets)
+        body
+      `(cl-flet ,(mapcan (lambda (ruleset)
+                           (let ((aliases (get ruleset 'peg--rules)))
+                             (unless aliases
+                               (message "Unknown PEG ruleset: %S" ruleset))
+                             (copy-sequence aliases)))
+                         rulesets)
+         ,body))))
 
 ;;;;; Old entry points
 

bug#74018: 31.0.50; Issue with define-peg-ruleset?

[Andrea Corallo]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> Package: Emacs
> Version: 31.0.50
>
>
> Augusto pointed out a bug in `peg.el` when you have code like:
>
>     (define-peg-ruleset myrules
>       (sign  () (or "+" "-" ""))
>       (digit () [0-9])
>       (nat   () digit (* digit))
>       (int   () sign digit (* digit))
>       (float () int "." nat))
>
>     (with-peg-rules
>         (myrules
>          (complex float "+i" float))
>       (peg-parse nat "," nat "," complex) )
>
> where the macroexpanded code is wrong and leads to errors like:
>
>     Debugger entered--Lisp error: (void-function peg-rule\ nat)
>
> I was about to install the bugfix below to `master` but noticed that it
> probably belongs to `emacs-30` instead since the bug makes
> `define-peg-ruleset` basically unusable and PEG is new in Emacs-30.
>
> Any objection?

No objections on my side.

Thanks

  Andrea
  

bug#74018: 31.0.50; Issue with define-peg-ruleset?

[Eli Zaretskii]

> Cc: monnier@iro.umontreal.ca
> Date: Fri, 25 Oct 2024 16:30:30 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Version: 31.0.50
           ^^^^^^^
Given what you say below, this is inaccurate.

> Augusto pointed out a bug in `peg.el` when you have code like:
> 
>     (define-peg-ruleset myrules
>       (sign  () (or "+" "-" ""))
>       (digit () [0-9])
>       (nat   () digit (* digit))
>       (int   () sign digit (* digit))
>       (float () int "." nat))
> 
>     (with-peg-rules
>         (myrules
>          (complex float "+i" float))
>       (peg-parse nat "," nat "," complex) )
> 
> where the macroexpanded code is wrong and leads to errors like:
> 
>     Debugger entered--Lisp error: (void-function peg-rule\ nat)
> 
> I was about to install the bugfix below to `master` but noticed that it
> probably belongs to `emacs-30` instead since the bug makes
> `define-peg-ruleset` basically unusable and PEG is new in Emacs-30.
> 
> Any objection?

I know nothing about peg.el, and you haven't explained the root cause
for the bug for me to understand why it "makes `define-peg-ruleset'
basically unusable", so let's hear from Stefan and Andrea first.

bug#74018: 31.0.50; Issue with define-peg-ruleset?

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

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

>> Version: 31.0.50
>            ^^^^^^^
> Given what you say below, this is inaccurate.

Indeed, sorry, I forgot to erase that info.

>> Augusto pointed out a bug in `peg.el` when you have code like:
>> 
>>     (define-peg-ruleset myrules
>>       (sign  () (or "+" "-" ""))
>>       (digit () [0-9])
>>       (nat   () digit (* digit))
>>       (int   () sign digit (* digit))
>>       (float () int "." nat))
>> 
>>     (with-peg-rules
>>         (myrules
>>          (complex float "+i" float))
>>       (peg-parse nat "," nat "," complex) )
>> 
>> where the macroexpanded code is wrong and leads to errors like:
>> 
>>     Debugger entered--Lisp error: (void-function peg-rule\ nat)
>> 
>> I was about to install the bugfix below to `master` but noticed that it
>> probably belongs to `emacs-30` instead since the bug makes
>> `define-peg-ruleset` basically unusable and PEG is new in Emacs-30.
>> 
>> Any objection?
>
> I know nothing about peg.el, and you haven't explained the root cause
> for the bug for me to understand why it "makes `define-peg-ruleset'
> basically unusable", so let's hear from Stefan and Andrea first.

Here's a more complete description.


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-with-peg-rules-Fix-references-to-rulesets-bug-74018.patch --]
[-- Type: text/x-diff, Size: 2533 bytes --]

From b9d3b8537418985a99dc8e6a513f8dd28d262339 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Sat, 26 Oct 2024 10:23:01 -0400
Subject: [PATCH] (with-peg-rules): Fix references to rulesets (bug#74018)

PEG rules get "compiled" to functions with name `peg-rule <RULE>`.
`define-peg-ruleset` instead defines it PEG rules with name
`peg-rule <RULESET> <RULE>`, so that they can be made visible
by `with-peg-rules` simply by adding local aliases from
`peg-rule <RULE>` to `peg-rule <RULESET> <RULE>`.

Apparently when I added `define-peg-ruleset` I somehow failed to
install some of the corresponding code in `with-peg-rules`, so
the aliases were not installed, making it "impossible" to use
rulesets.
[ I still have no idea how this happened and/or where
  the missing code went, so I recreated it.  ]

* lisp/progmodes/peg.el (with-peg-rules): Install the aliases
for the rulesets.
---
 lisp/progmodes/peg.el | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/peg.el b/lisp/progmodes/peg.el
index 96334162195..0e6ba9318e3 100644
--- a/lisp/progmodes/peg.el
+++ b/lisp/progmodes/peg.el
@@ -412,6 +412,7 @@ define-peg-ruleset
              (full-rname (format "%s %s" name rname)))
         (push `(define-peg-rule ,full-rname . ,(cdr rule)) defs)
         (push `(,(peg--rule-id rname) #',(peg--rule-id full-rname)) aliases)))
+    (require 'cl-lib)
     `(cl-flet ,aliases
        ,@defs
        (eval-and-compile (put ',name 'peg--rules ',aliases)))))
@@ -432,7 +433,8 @@ with-peg-rules
                               (progn (push rule rulesets) nil)
                             (cons (car rule) (peg-normalize `(and . ,(cdr rule))))))
                         rules)))
-        (ctx (assq :peg-rules macroexpand-all-environment)))
+         (ctx (assq :peg-rules macroexpand-all-environment))
+         (body
     (macroexpand-all
      `(cl-labels
           ,(mapcar (lambda (rule)
@@ -444,6 +446,15 @@ with-peg-rules
         ,@body)
      `((:peg-rules ,@(append rules (cdr ctx)))
        ,@macroexpand-all-environment))))
+    (if (null rulesets)
+        body
+      `(cl-flet ,(mapcan (lambda (ruleset)
+                           (let ((aliases (get ruleset 'peg--rules)))
+                             (unless aliases
+                               (message "Unknown PEG ruleset: %S" ruleset))
+                             (copy-sequence aliases)))
+                         rulesets)
+         ,body))))
 
 ;;;;; Old entry points
 
-- 
2.39.5

bug#74018: 31.0.50; Issue with define-peg-ruleset?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Stefan Kangas <stefankangas@gmail.com>,  Andrea Corallo
>  <acorallo@gnu.org>,  74018@debbugs.gnu.org
> Date: Sat, 26 Oct 2024 10:24:33 -0400
> 
> >> I was about to install the bugfix below to `master` but noticed that it
> >> probably belongs to `emacs-30` instead since the bug makes
> >> `define-peg-ruleset` basically unusable and PEG is new in Emacs-30.
> >> 
> >> Any objection?
> >
> > I know nothing about peg.el, and you haven't explained the root cause
> > for the bug for me to understand why it "makes `define-peg-ruleset'
> > basically unusable", so let's hear from Stefan and Andrea first.
> 
> Here's a more complete description.

Thanks.  Andrea already agreed with installing this on the release
branch.  I hope Stefan Kangas will respond soon.

bug#74018: 31.0.50; Issue with define-peg-ruleset?

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: Stefan Kangas <stefankangas@gmail.com>,  Andrea Corallo
>>  <acorallo@gnu.org>,  74018@debbugs.gnu.org
>> Date: Sat, 26 Oct 2024 10:24:33 -0400
>>
>> >> I was about to install the bugfix below to `master` but noticed that it
>> >> probably belongs to `emacs-30` instead since the bug makes
>> >> `define-peg-ruleset` basically unusable and PEG is new in Emacs-30.
>> >>
>> >> Any objection?
>> >
>> > I know nothing about peg.el, and you haven't explained the root cause
>> > for the bug for me to understand why it "makes `define-peg-ruleset'
>> > basically unusable", so let's hear from Stefan and Andrea first.
>>
>> Here's a more complete description.
>
> Thanks.  Andrea already agreed with installing this on the release
> branch.  I hope Stefan Kangas will respond soon.

No objections from me either, please go ahead.

bug#74018: 31.0.50; Issue with define-peg-ruleset?

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

> No objections from me either, please go ahead.

Pushed to `emacs-30`, thanks,


        Stefan