GNU bug report logs - #25965
26.0.50; iter-lambda may evaluate certain forms twice

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 25965 in the body.
You can then email your comments to 25965 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: Andreas Politz <politza <at> hochschule-trier.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.50; iter-lambda may evaluate certain forms twice 
Date: Sat, 04 Mar 2017 09:02:32 +0100

Here is an example where the behavior occurs.

#+BEGIN_SRC emacs-lisp
  ;; -*- lexical-binding: t -*-
  (iter-next
   (funcall (iter-lambda ()
              (let* ((fill-column 10) ;;any special variable will do
                     (i 0)
                     (j (setq i (1+ i))))
                (iter-yield i)))))
  ;; => 2
#+END_SRC

But the result should be 1.  Looking at the expanded code, we see i
incremented indeed twice.

#+BEGIN_SRC emacs-lisp
  ;; ....
  (setq cps-state-atom--1522
        #'(lambda nil
            (setq cps-current-value--1513
                  (let
                      ((fill-column cps-binding-fill-column--1516))
                    (unwind-protect
                        (prog1
                            (setq cps-binding-i--1517
                                  (1+ cps-binding-i--1517))
                          (setq cps-current-state--1514 cps-state-let*--1521))
                      (setq cps-binding-fill-column--1516 fill-column))
                    (prog1
                        (setq cps-binding-i--1517
                              (1+ cps-binding-i--1517))
                      (setq cps-current-state--1514 cps-state-let*--1521))))))
  ;; ...
#+END_SRC

The unwind-protect form is created in this function.

#+BEGIN_SRC emacs-lisp
  (defun cps--make-dynamic-binding-wrapper (dynamic-var static-var)
    (cl-assert lexical-binding)
    (lambda (form)
      `(let ((,dynamic-var ,static-var))
         (unwind-protect ; Update the static shadow after evaluation is done
              ,form
           (setf ,static-var ,dynamic-var))
         ,form)))
#+END_SRC

And it seems to me that the second occurrence of ,form is just an
error.

By the way, why is debugging these generators made so difficult (via the
implementation of cps--gensym) ?

-ap

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Andreas Politz <politza <at> hochschule-trier.de>,
 Daniel Colascione <dancol <at> dancol.org>
Cc: 25965 <at> debbugs.gnu.org
Subject: Re: bug#25965: 26.0.50; iter-lambda may evaluate certain forms twice
Date: Wed, 08 Mar 2017 16:52:13 -0500

> The unwind-protect form is created in this function.
>
> #+BEGIN_SRC emacs-lisp
>   (defun cps--make-dynamic-binding-wrapper (dynamic-var static-var)
>     (cl-assert lexical-binding)
>     (lambda (form)
>       `(let ((,dynamic-var ,static-var))
>          (unwind-protect ; Update the static shadow after evaluation is done
>               ,form
>            (setf ,static-var ,dynamic-var))
>          ,form)))
> #+END_SRC
>
> And it seems to me that the second occurrence of ,form is just an
> error.

Hmm... indeed that looks odd, and maybe just removing the second ,form
is the right fix.  Daniel, do you happen to remember what the second
,form above is meant to do?


        Stefan

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

From: Daniel Colascione <dancol <at> dancol.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 25965 <at> debbugs.gnu.org, Andreas Politz <politza <at> hochschule-trier.de>
Subject: Re: bug#25965: 26.0.50; iter-lambda may evaluate certain forms twice
Date: Wed, 08 Mar 2017 14:08:07 -0800

On Wed, Mar 08 2017, Stefan Monnier wrote:
>> The unwind-protect form is created in this function.
>>
>> #+BEGIN_SRC emacs-lisp
>>   (defun cps--make-dynamic-binding-wrapper (dynamic-var static-var)
>>     (cl-assert lexical-binding)
>>     (lambda (form)
>>       `(let ((,dynamic-var ,static-var))
>>          (unwind-protect ; Update the static shadow after evaluation is done
>>               ,form
>>            (setf ,static-var ,dynamic-var))
>>          ,form)))
>> #+END_SRC
>>
>> And it seems to me that the second occurrence of ,form is just an
>> error.
>
> Hmm... indeed that looks odd, and maybe just removing the second ,form
> is the right fix.  Daniel, do you happen to remember what the second
> ,form above is meant to do?

Embarrass me years later? :-) I think it's safe to remove it. Let's make
sure to add a test too.

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

From: Gemini Lasswell <gazally <at> runbox.com>
To: Daniel Colascione <dancol <at> dancol.org>
Cc: 25965 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> IRO.UMontreal.CA>,
 Andreas Politz <politza <at> hochschule-trier.de>
Subject: Re: bug#25965: 26.0.50; iter-lambda may evaluate certain forms twice
Date: Thu, 21 Sep 2017 14:36:34 -0700

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

tags 25965 patch
quit

Daniel Colascione <dancol <at> dancol.org> writes:

> I think it's safe to remove it. Let's make
> sure to add a test too.

Here's a patch which removes the second ,form and adds a test:

[0001-Fix-dynamic-binding-wrapper-in-iter-lambda-bug-25965.patch (text/plain, inline)]

From 46720f235ff11f64a35b91e74110fd3a8e01be4f Mon Sep 17 00:00:00 2001
From: Gemini Lasswell <gazally <at> runbox.com>
Date: Wed, 20 Sep 2017 10:17:35 -0700
Subject: [PATCH] Fix dynamic binding wrapper in iter-lambda (bug#25965)

* lisp/emacs-lisp/generator.el (cps--make-dynamic-binding-wrapper):
Remove extra evaluation of form.
* test/lisp/emacs-lisp/generator-tests.el
(cps-iter-lambda-with-dynamic-binding): New test.
---
 lisp/emacs-lisp/generator.el            |  3 +--
 test/lisp/emacs-lisp/generator-tests.el | 10 ++++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/generator.el b/lisp/emacs-lisp/generator.el
index f3597cc387..3e9885900c 100644
--- a/lisp/emacs-lisp/generator.el
+++ b/lisp/emacs-lisp/generator.el
@@ -142,8 +142,7 @@ cps--make-dynamic-binding-wrapper
     `(let ((,dynamic-var ,static-var))
        (unwind-protect ; Update the static shadow after evaluation is done
             ,form
-         (setf ,static-var ,dynamic-var))
-       ,form)))
+         (setf ,static-var ,dynamic-var)))))
 
 (defmacro cps--with-dynamic-binding (dynamic-var static-var &rest body)
   "Evaluate BODY such that generated atomic evaluations run with
diff --git a/test/lisp/emacs-lisp/generator-tests.el b/test/lisp/emacs-lisp/generator-tests.el
index 4cc6c841da..cbb136ae91 100644
--- a/test/lisp/emacs-lisp/generator-tests.el
+++ b/test/lisp/emacs-lisp/generator-tests.el
@@ -282,3 +282,13 @@ cps-test-closed-flag
 (ert-deftest cps-test-declarations-preserved ()
   (should (equal (documentation 'generator-with-docstring) "Documentation!"))
   (should (equal (get 'generator-with-docstring 'lisp-indent-function) 5)))
+
+(ert-deftest cps-iter-lambda-with-dynamic-binding ()
+  "`iter-lambda' with dynamic binding produces correct result (bug#25965)."
+  (should (= 1
+             (iter-next
+              (funcall (iter-lambda ()
+                         (let* ((fill-column 10) ;;any special variable will do
+                                (i 0)
+                                (j (setq i (1+ i))))
+                           (iter-yield i))))))))
-- 
2.14.1

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

From: Gemini Lasswell <gazally <at> runbox.com>
To: Daniel Colascione <dancol <at> dancol.org>
Cc: 25965-done <at> debbugs.gnu.org, Stefan Monnier <monnier <at> IRO.UMontreal.CA>,
 Andreas Politz <politza <at> hochschule-trier.de>
Subject: Re: bug#25965: 26.0.50; iter-lambda may evaluate certain forms twice
Date: Thu, 05 Oct 2017 12:51:55 -0700

Pushed to emacs-26.

Previous Next

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