bug#19613: 25.0.50; cl-labels bug

bug#19613: 25.0.50; cl-labels bug

[Katsumi Yamaoka]

Hi,

This form got not to work recently:

(cl-labels ((fn (arg) arg))
  (apply #'fn (list "arg")))

It is because `fn' is not substituted with the one CL defines:

(let (--cl-fn--)
  (setq --cl-fn-- #'(lambda (arg) arg))
  (apply #'fn (list "arg")))

Thanks.

In GNU Emacs 25.0.50.1 (i686-pc-cygwin, GTK+ Version 3.10.9)
 of 2015-01-16 on localhost
Windowing system distributor `The Cygwin/X Project', version 11.0.11501000
Configured using:
 `configure --verbose --with-x-toolkit=gtk3'

bug#19613: 25.0.50; cl-labels bug

[Stefan Monnier]

> This form got not to work recently:

> (cl-labels ((fn (arg) arg))
>   (apply #'fn (list "arg")))

> It is because `fn' is not substituted with the one CL defines:

> (let (--cl-fn--)
>   (setq --cl-fn-- #'(lambda (arg) arg))
>   (apply #'fn (list "arg")))

I believe this was fixed yesterday, even before you filed the bug.
Could you double check?


        Stefan

bug#19613: 25.0.50; cl-labels bug

[Glenn Morris]

Stefan Monnier wrote:

> I believe this was fixed yesterday, even before you filed the bug.
> Could you double check?

No, it still fails.

bug#19613: 25.0.50; cl-labels bug

[Stefan Monnier]

>> I believe this was fixed yesterday, even before you filed the bug.
>> Could you double check?
> No, it still fails.

I believe this time it is fixed,


        Stefan

bug#19613: 25.0.50; cl-labels bug

[Katsumi Yamaoka]

On Fri, 16 Jan 2015 22:57:27 -0500, Stefan Monnier wrote:
> I believe this time it is fixed,

I have still a problem with `cl-labels' used within `lexical-let'.

(lexical-let (var)
  (cl-labels ((fn (arg) arg))
    (apply #'fn (list "arg"))))
 => apply: Symbol's function definition is void: fn

Emacs-w3m uses this kind of macros.  Thanks.

bug#19613: 25.0.50; cl-labels bug

[Katsumi Yamaoka]

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

On Thu, 22 Jan 2015 16:07:17 +0900, Katsumi Yamaoka wrote:
> I have still a problem with `cl-labels' used within `lexical-let'.

> (lexical-let (var)
>   (cl-labels ((fn (arg) arg))
>     (apply #'fn (list "arg"))))
>  => apply: Symbol's function definition is void: fn

When a `cl-labels' form is wrapped with `lexical-let', it runs
`cl--function-convert' instead of `cl--labels-convert'.
So, `cl--function-convert' also needs to be fixed like the ones
Stefan did in `cl--labels-convert'[1], doesn't it?  I tried the
attached patch and verified it fixes not only the `cl-labels'
bug but also the `cl-flet' bug[2].

[1] git diff -U 9d940c6 69f36af lisp/emacs-lisp/cl-macs.el
[2]
(lexical-let (var)
  (cl-flet ((fn (arg) arg))
    (apply #'fn (list "arg"))))
 => apply: Symbol's function definition is void: fn


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

--- cl.el~	2015-01-26 04:16:45.291325300 +0000
+++ cl.el	2015-01-26 05:53:38.810655900 +0000
@@ -374,10 +374,12 @@
           (setq cl--function-convert-cache (cons newf res))
           res))))
    (t
-    (let ((found (assq f macroexpand-all-environment)))
-      (if (and found (ignore-errors
-                       (eq (cadr (cl-caddr found)) 'cl-labels-args)))
-          (cadr (cl-caddr (cl-cadddr found)))
+    (let* ((found (assq f macroexpand-all-environment))
+           (replacement (and found
+                             (ignore-errors
+                               (funcall (cdr found) cl--labels-magic)))))
+      (if (and replacement (eq cl--labels-magic (car replacement)))
+          (nth 1 replacement)
         (let ((res `(function ,f)))
           (setq cl--function-convert-cache (cons f res))
           res))))))

bug#19613: 25.0.50; cl-labels bug

[Stefan Monnier]

> -    (let ((found (assq f macroexpand-all-environment)))
> -      (if (and found (ignore-errors
> -                       (eq (cadr (cl-caddr found)) 'cl-labels-args)))
> -          (cadr (cl-caddr (cl-cadddr found)))
> +    (let* ((found (assq f macroexpand-all-environment))
> +           (replacement (and found
> +                             (ignore-errors
> +                               (funcall (cdr found) cl--labels-magic)))))
> +      (if (and replacement (eq cl--labels-magic (car replacement)))
> +          (nth 1 replacement)
>          (let ((res `(function ,f)))
>            (setq cl--function-convert-cache (cons f res))
>            res))))))

This looks correct.  Ideally this part of cl--function-convert should
delegate to cl--labels-convert to avoid the code duplication, tho.


        Stefan

bug#19613: 25.0.50; cl-labels bug

[Katsumi Yamaoka]

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

On Mon, 26 Jan 2015 10:14:56 -0500, Stefan Monnier wrote:
[...]
> This looks correct.  Ideally this part of cl--function-convert should
> delegate to cl--labels-convert to avoid the code duplication, tho.

That's much better.  Here it is:

[-- Attachment #2: Type: text/x-patch, Size: 1136 bytes --]

--- cl.el~	2015-01-26 04:16:45.291325300 +0000
+++ cl.el	2015-01-26 22:57:02.040588600 +0000
@@ -342,6 +342,7 @@
 - renaming of F when it's a function defined via `cl-labels' or `labels'."
   (require 'cl-macs)
   (declare-function cl--expr-contains-any "cl-macs" (x y))
+  (declare-function cl--labels-convert "cl-macs" (f))
   (cond
    ;; ¡¡Big Ugly Hack!! We can't use a compiler-macro because those are checked
    ;; *after* handling `function', but we want to stop macroexpansion from
@@ -374,13 +375,8 @@
           (setq cl--function-convert-cache (cons newf res))
           res))))
    (t
-    (let ((found (assq f macroexpand-all-environment)))
-      (if (and found (ignore-errors
-                       (eq (cadr (cl-caddr found)) 'cl-labels-args)))
-          (cadr (cl-caddr (cl-cadddr found)))
-        (let ((res `(function ,f)))
-          (setq cl--function-convert-cache (cons f res))
-          res))))))
+    (setq cl--labels-convert-cache cl--function-convert-cache)
+    (cl--labels-convert f))))
 
 (defmacro lexical-let (bindings &rest body)
   "Like `let', but lexically scoped.

bug#19613: 25.0.50; cl-labels bug

[Katsumi Yamaoka]

On Tue, 27 Jan 2015 08:00:57 +0900, Katsumi Yamaoka wrote:
> --- cl.el~	2015-01-26 04:16:45.291325300 +0000
> +++ cl.el	2015-01-26 22:57:02.040588600 +0000
> @@ -342,6 +342,7 @@
>  - renaming of F when it's a function defined via `cl-labels' or `labels'."
>    (require 'cl-macs)
>    (declare-function cl--expr-contains-any "cl-macs" (x y))
> +  (declare-function cl--labels-convert "cl-macs" (f))

There needs to be:
  +  (defvar cl--labels-convert-cache)

bug#19613: 25.0.50; cl-labels bug

[Stefan Monnier]

> That's much better.  Here it is:

Looks great, thanks,


        Stefan

bug#19613: 25.0.50; cl-labels bug

[Katsumi Yamaoka]

On Mon, 26 Jan 2015 21:06:16 -0500, Stefan Monnier wrote:
> Looks great, thanks,

Thanks.  Committed.