GNU bug report logs - #26068
[PATCH] bug in generator function with pcase

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 26068 in the body.
You can then email your comments to 26068 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: Paul Pogonyshev <pogonyshev <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] bug in generator function with pcase
Date: Sun, 12 Mar 2017 09:51:23 +0100

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

To reproduce:

(iter-next (funcall (iter-lambda () (pcase (list 1 2)
                                      (`(,a ,b) (iter-yield (+ a b)))))))

Patch is attached. I have no idea if it affects something else, but
matching symbols with `eq' on their names (`eq' on strings?!) seems
outright wrong to me.

On a side note: bug report I sent yesterday with M-x report-emacs-bug
was silently lost.

Paul

[cl-macs.diff (text/plain, attachment)]

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

From: Mark Oteiza <mvoteiza <at> udel.edu>
To: Paul Pogonyshev <pogonyshev <at> gmail.com>
Cc: 26068-done <at> debbugs.gnu.org
Subject: Re: bug#26068: [PATCH] bug in generator function with pcase
Date: Sun, 19 Mar 2017 15:18:47 -0400

Paul Pogonyshev <pogonyshev <at> gmail.com> writes:

> To reproduce:
>
> (iter-next (funcall (iter-lambda () (pcase (list 1 2)
>                                       (`(,a ,b) (iter-yield (+ a b)))))))
>
> Patch is attached. I have no idea if it affects something else, but
> matching symbols with `eq' on their names (`eq' on strings?!) seems
> outright wrong to me.
>
> On a side note: bug report I sent yesterday with M-x report-emacs-bug
> was silently lost.

Applied as 0d112c00.

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Paul Pogonyshev <pogonyshev <at> gmail.com>
Cc: Mark Oteiza <mvoteiza <at> udel.edu>, tino.calancha <at> gmail.com,
 26068 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#26068: [PATCH] bug in generator function with pcase
Date: Sat, 01 Apr 2017 22:08:57 +0900

Paul Pogonyshev <pogonyshev <at> gmail.com> writes:

> To reproduce:
>
> (iter-next (funcall (iter-lambda () (pcase (list 1 2)
>                                       (`(,a ,b) (iter-yield (+ a b)))))))
>
> Patch is attached. I have no idea if it affects something else, but
> matching symbols with `eq' on their names (`eq' on strings?!) seems
> outright wrong to me.
It looks also weird to me.  It seems written on purpose in order to
not expand fuction names in `cl-symbol-macrolet'.

Stefan has added a new test 2 days ago which fails after your
patch:
* test/lisp/emacs-lisp/cl-lib-tests.el (cl-lib-symbol-macrolet)

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

From: Paul Pogonyshev <pogonyshev <at> gmail.com>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Mark Oteiza <mvoteiza <at> udel.edu>, 26068 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#26068: [PATCH] bug in generator function with pcase
Date: Sat, 1 Apr 2017 15:44:16 +0200

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

Attached is the testcase from the bug as a patch. It might be best to
reformulate it without `pcase' though, to avoid depending on its
implementation details.

The testcase fails before the patch, passes now. So, I don't know
anything about `cl-flet', but the patch was not without reasons.

Paul

On 1 April 2017 at 15:08, Tino Calancha <tino.calancha <at> gmail.com> wrote:
> Paul Pogonyshev <pogonyshev <at> gmail.com> writes:
>
>> To reproduce:
>>
>> (iter-next (funcall (iter-lambda () (pcase (list 1 2)
>>                                       (`(,a ,b) (iter-yield (+ a b)))))))
>>
>> Patch is attached. I have no idea if it affects something else, but
>> matching symbols with `eq' on their names (`eq' on strings?!) seems
>> outright wrong to me.
> It looks also weird to me.  It seems written on purpose in order to
> not expand fuction names in `cl-symbol-macrolet'.
>
> Stefan has added a new test 2 days ago which fails after your
> patch:
> * test/lisp/emacs-lisp/cl-lib-tests.el (cl-lib-symbol-macrolet)

[pcase-generator-test.diff (text/plain, attachment)]

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Paul Pogonyshev <pogonyshev <at> gmail.com>
Cc: Mark Oteiza <mvoteiza <at> udel.edu>, 26068 <at> debbugs.gnu.org,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#26068: [PATCH] bug in generator function with pcase
Date: Sat, 01 Apr 2017 12:12:15 -0400

> +;; Bug #26068.  The problem is that `pcase' uses several `x' symbols,
> +;; which are not identical, but have the same name.
> +(ert-deftest cps-test-symbols-with-same-name ()
> +  (should (equal (iter-next (funcall (iter-lambda () (pcase (list 1 2)
> +                                                       (`(,a ,b) (iter-yield (+ a b)))))))
> +                 3)))

Right.  I guess the core if the issue would be reproduced with something like

    (defmacro cl-lib-symbol-macrolet-4+5 ()
      (let* ((sname "x")
             (s1 (make-symbol sname))
             (s2 (make-symbol sname)))
        `(cl-symbol-macrolet ((,s1 4)
                              (,s2 5))
           (+ ,s1 ,s2))))
    (ert-deftest cl-lib-symbol-macrolet-2 ()
      (should (equal (cl-lib-symbol-macrolet-4+5) (+ 4 5))))

Before your patch, the macro's code returned 10 rather than 9.

I installed this test in cl-lib-tests.el.


        Stefan

Previous Next

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