unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#31792: 27.0.50; Regression in #'labels, recent versions
@ 2018-06-11 23:11 Aidan Kehoe
  2018-06-11 23:31 ` Noam Postavsky
       [not found] ` <mailman.1699.1528759928.1292.bug-gnu-emacs@gnu.org>
  0 siblings, 2 replies; 11+ messages in thread
From: Aidan Kehoe @ 2018-06-11 23:11 UTC (permalink / raw)
  To: 31792


When I start this binary (see below for the details), do M-x byte-compile-file
/tmp/aidan/u.el RET, and then execute the following in *scratch*:

(progn (load "/tmp/aidan/u.elc")
       (symbol-function 'my-package-accept-process-output))

I get:

=> my-package-accept-process-output-1

When I do the same in the emacs 22.1.1 that Apple built and shipped with
my OS, I get a compiled function object, as is much closer to being the
correct behaviour.

Contents of /tmp/aidan/u.el (a narrowed-down test case of some code I had
planned to write): 

---- Begin:

(require 'cl)

(labels
    ((my-package-accept-process-output-1 (process-object &optional timeout-secs
                                                     timeout-msecs)
       "Call `my-package-filter' and pass it the output of the last my-package
invocation."
       (if (null process-object)
           (error "No My-Package process to read output from!")
         (let ((buf my-package-output-buffer)
               my-package-output)
           (if (not (bufferp buf))
               (setq my-package-filter nil)
             (with-current-buffer buf
               (setq my-package-output (buffer-substring-no-properties
                                    (point-min) (point-max))))
             (my-package-filter t my-package-output)
             (with-current-buffer buf
               (erase-buffer)))))))
  (defalias 'my-package-accept-process-output
      (if (boundp 'my-package-async-processp)
          #'accept-process-output
        #'my-package-accept-process-output-1)))

---- End:



In GNU Emacs 27.0.50 (build 3, i386-apple-darwin10.8.0, NS appkit-1038.36 Version 10.6.8 (Build 10K549))
 of 2018-06-11 built on bonbon
Repository revision: 94d60f59fc654706c3a52ed2c90c355b36be7898
Windowing system distributor 'Apple', version 10.3.1038
System Description:  Mac OS X 10.6.8

Recent messages:
Auto-saving...done
Saving file /tmp/aidan/u.el...
Wrote /tmp/aidan/u.el
Compiling /tmp/aidan/u.el...
‘labels’ is an obsolete macro (as of 24.3); use ‘cl-labels’ instead.
Compiling /tmp/aidan/u.el...done
Wrote /tmp/aidan/u.elc
Loading /tmp/aidan/u.elc...done
Quit
Mark set [3 times]

Configured using:
 'configure --with-wide-int CC=gcc-4.2
 PKG_CONFIG_PATH=/usr/pkg/lib/pkgconfig:/X11/lib/pkgconfig'

Configured features:
RSVG IMAGEMAGICK DBUS NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS
NS THREADS LCMS2

Important settings:
  value of $LANG: de_DE.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq dired
dired-loaddefs format-spec rfc822 mml mml-sec password-cache epa derived
epg epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils cl-seq cl-macs
disass cl-extra cl gv help-fns radix-tree help-mode easymenu cl-loaddefs
cl-lib warnings byte-opt compile comint ansi-color ring bytecomp
byte-compile cconv time-date elec-pair tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/ns-win ns-win
ucs-normalize mule-util term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind kqueue cocoa ns lcms2 multi-tty make-network-process
emacs)

Memory information:
((conses 16 225419 15013)
 (symbols 40 21102 1)
 (miscs 36 68 413)
 (strings 16 33618 3363)
 (string-bytes 1 872171)
 (vectors 12 37343)
 (vector-slots 8 742671 16894)
 (floats 8 49 351)
 (intervals 36 910 74)
 (buffers 860 18))

-- 
‘As I sat looking up at the Guinness ad, I could never figure out /
How your man stayed up on the surfboard after forty pints of stout’
(C. Moore)





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#31792: 27.0.50; Regression in #'labels, recent versions
  2018-06-11 23:11 bug#31792: 27.0.50; Regression in #'labels, recent versions Aidan Kehoe
@ 2018-06-11 23:31 ` Noam Postavsky
  2018-06-12 22:58   ` Noam Postavsky
       [not found] ` <mailman.1699.1528759928.1292.bug-gnu-emacs@gnu.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Noam Postavsky @ 2018-06-11 23:31 UTC (permalink / raw)
  To: Aidan Kehoe; +Cc: 31792

found 31792 25.3
quit

Aidan Kehoe <kehoea@parhasard.net> writes:

> When I do the same in the emacs 22.1.1 that Apple built and shipped with
> my OS, I get a compiled function object, as is much closer to being the
> correct behaviour.

Seems to have regressed in Emacs 25, cl-labels still works correctly.
Here's a smaller reproducer:

(labels ((foo () t))
  #'foo) ;=> foo

(cl-labels ((foo () t))
  #'foo) ;=> (lambda nil t)





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#31792: 27.0.50; Regression in #'labels, recent versions
  2018-06-11 23:31 ` Noam Postavsky
@ 2018-06-12 22:58   ` Noam Postavsky
  2018-06-13 13:16     ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Noam Postavsky @ 2018-06-12 22:58 UTC (permalink / raw)
  To: Aidan Kehoe; +Cc: 31792, Stefan Monnier

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

tags 31792 + patch
quit

> Seems to have regressed in Emacs 25, cl-labels still works correctly.
> Here's a smaller reproducer:
>
> (labels ((foo () t))
>   #'foo) ;=> foo
>
> (cl-labels ((foo () t))
>   #'foo) ;=> (lambda nil t)

It looks like `labels' missed the update in [1: 69f36afa11], here's a
patch to apply it.  It's not immediately clear to me if the requirement
to use a backquoted lambda vs closure is based on using lexical scoping
where the macro is defined or where it's used, so I left that as is.
Should be fine for emacs-26, I think.


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

From aa197176d0de4a6b7d1ee5d16d0fc1750f7237cf Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Tue, 12 Jun 2018 18:41:46 -0400
Subject: [PATCH] Fix #'fun handling inside `labels' (Bug#31792)

* lisp/emacs-lisp/cl.el (labels): Apply the equivalent of the
cl-labels change from 2015-01-16 "* lisp/emacs-lisp/cl-macs.el: Fix
last change".
---
 lisp/emacs-lisp/cl.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/cl.el b/lisp/emacs-lisp/cl.el
index d53c8e0bbc..85deb8cb4f 100644
--- a/lisp/emacs-lisp/cl.el
+++ b/lisp/emacs-lisp/cl.el
@@ -466,8 +466,9 @@ labels
 	(push var sets)
 	(push (cons (car binding)
                     `(lambda (&rest cl-labels-args)
-                       (cl-list* 'funcall ',var
-                                 cl-labels-args)))
+                       (if (eq (car cl-labels-args) cl--labels-magic)
+                           (list cl--labels-magic ',var)
+                         (cl-list* 'funcall ',var cl-labels-args))))
               newenv)))
     (macroexpand-all `(lexical-let ,vars (setq ,@sets) ,@body) newenv)))
 
-- 
2.11.0


[-- Attachment #3: Type: text/plain, Size: 191 bytes --]


[1: 69f36afa11]: 2015-01-16 17:49:00 -0500
  * lisp/emacs-lisp/cl-macs.el: Fix last change.
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=69f36afa11c0b754c40f4fc57408ccd85428e2b0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#31792: 27.0.50; Regression in #'labels, recent versions
  2018-06-12 22:58   ` Noam Postavsky
@ 2018-06-13 13:16     ` Stefan Monnier
  2018-06-14  3:24       ` Noam Postavsky
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2018-06-13 13:16 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Aidan Kehoe, 31792

> diff --git a/lisp/emacs-lisp/cl.el b/lisp/emacs-lisp/cl.el
> index d53c8e0bbc..85deb8cb4f 100644
> --- a/lisp/emacs-lisp/cl.el
> +++ b/lisp/emacs-lisp/cl.el
> @@ -466,8 +466,9 @@ labels
>  	(push var sets)
>  	(push (cons (car binding)
>                      `(lambda (&rest cl-labels-args)
> -                       (cl-list* 'funcall ',var
> -                                 cl-labels-args)))
> +                       (if (eq (car cl-labels-args) cl--labels-magic)
> +                           (list cl--labels-magic ',var)
> +                         (cl-list* 'funcall ',var cl-labels-args))))
>                newenv)))
>      (macroexpand-all `(lexical-let ,vars (setq ,@sets) ,@body) newenv)))

Looks good, but please add a comment before the call to macroexpand-all
reminding that lexical-let installs a macroexpander for `function` that
ends up calling cl--labels-convert.


        Stefan





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#31792: 27.0.50; Regression in #'labels, recent versions
       [not found] ` <mailman.1699.1528759928.1292.bug-gnu-emacs@gnu.org>
@ 2018-06-13 16:57   ` Alan Mackenzie
  2018-06-13 17:03     ` Noam Postavsky
  0 siblings, 1 reply; 11+ messages in thread
From: Alan Mackenzie @ 2018-06-13 16:57 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 31792

In article <mailman.1699.1528759928.1292.bug-gnu-emacs@gnu.org> you wrote:
> found 31792 25.3
> quit

> Aidan Kehoe <kehoea@parhasard.net> writes:

>> When I do the same in the emacs 22.1.1 that Apple built and shipped with
>> my OS, I get a compiled function object, as is much closer to being the
>> correct behaviour.

> Seems to have regressed in Emacs 25, cl-labels still works correctly.
> Here's a smaller reproducer:

> (labels ((foo () t))
>   #'foo) ;=> foo

> (cl-labels ((foo () t))
>   #'foo) ;=> (lambda nil t)

Just as a matter of interest, the doc strings for both these functions
are poor.

That for cl-labels says "make temporary function bindings" without saying
what a "function binding" is (it's not obvious), without saying what
functions (?or symbols) are being bound, and doesn't say whether they get
bound one after the other (in `let*' fashion) or all at once (in `let'
fashion).

It goes on to say "The bindings can be recursive, ...".  This is
gibberish to me.

Further, "the scoping is lexical".  The scoping of what is lexical?  And
in what?

I dare say I could fathom out most of these things with effort, but I
shouldn't have to.  Maybe this macro could be of use to me, but with the
doc string as it is, I'll never find out.  Pity.

-- 
Alan Mackenzie (Nuremberg, Germany).






^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#31792: 27.0.50; Regression in #'labels, recent versions
  2018-06-13 16:57   ` Alan Mackenzie
@ 2018-06-13 17:03     ` Noam Postavsky
  0 siblings, 0 replies; 11+ messages in thread
From: Noam Postavsky @ 2018-06-13 17:03 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 31792

On 13 June 2018 at 12:57, Alan Mackenzie <acm@muc.de> wrote:

>> (labels ((foo () t))
>>   #'foo) ;=> foo
>
>> (cl-labels ((foo () t))
>>   #'foo) ;=> (lambda nil t)
>
> Just as a matter of interest, the doc strings for both these functions
> are poor.

Yes; the manual's descriptions are much better.

(cl) Function Bindings
https://www.gnu.org/software/emacs/manual/html_node/cl/Function-Bindings.html





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#31792: 27.0.50; Regression in #'labels, recent versions
  2018-06-13 13:16     ` Stefan Monnier
@ 2018-06-14  3:24       ` Noam Postavsky
  2018-06-14  3:32         ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Noam Postavsky @ 2018-06-14  3:24 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Aidan Kehoe, 31792, Alan Mackenzie

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

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Looks good, but please add a comment before the call to macroexpand-all
> reminding that lexical-let installs a macroexpander for `function` that
> ends up calling cl--labels-convert.

Ok, I extended the docstrings of cl-labels and cl-flet a bit and added a
test too.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 4900 bytes --]

From c344010e7e2922a75769926896d585d367bbc5a8 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Tue, 12 Jun 2018 18:41:46 -0400
Subject: [PATCH] Fix #'fun handling inside `labels' (Bug#31792)

* lisp/emacs-lisp/cl.el (labels): Apply the equivalent of the
cl-labels change from 2015-01-16 "* lisp/emacs-lisp/cl-macs.el: Fix
last change".
* lisp/emacs-lisp/cl-macs.el (cl-flet, cl-labels): Improve docstring,
link to relevant manual page.
* test/lisp/emacs-lisp/cl-tests.el (labels-function-quoting): New
test.
---
 lisp/emacs-lisp/cl-macs.el       | 16 ++++++++++++----
 lisp/emacs-lisp/cl.el            |  7 +++++--
 test/lisp/emacs-lisp/cl-tests.el | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 6 deletions(-)
 create mode 100644 test/lisp/emacs-lisp/cl-tests.el

diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 9c47ceae18..e80efecac9 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -1964,13 +1964,17 @@ cl--labels-convert
 ;;;###autoload
 (defmacro cl-flet (bindings &rest body)
   "Make local function definitions.
-Like `cl-labels' but the definitions are not recursive.
+Bind the function cells of symbols within FORM.
 Each binding can take the form (FUNC EXP) where
 FUNC is the function name, and EXP is an expression that returns the
 function value to which it should be bound, or it can take the more common
 form \(FUNC ARGLIST BODY...) which is a shorthand
 for (FUNC (lambda ARGLIST BODY)).
 
+The bindings only take effect within FORM, not BODY, so you can't
+write recursive function definitions.  Use `cl-labels' for that.
+See info node `(cl) Function Bindings' for details.
+
 \(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
   (declare (indent 1) (debug ((&rest (cl-defun)) cl-declarations body)))
   (let ((binds ()) (newenv macroexpand-all-environment))
@@ -2012,9 +2016,13 @@ cl-flet*
 
 ;;;###autoload
 (defmacro cl-labels (bindings &rest body)
-  "Make temporary function bindings.
-The bindings can be recursive and the scoping is lexical, but capturing them
-in closures will only work if `lexical-binding' is in use.
+    "Make local function definitions.
+Bind the function cells of symbols within FORM.  Each binding can
+take the form (FUNC ARGLIST BODY...) where FUNC is the function
+name, ARGLIST its arguments, and BODY the forms of the function
+body.  The bindings can be referenced from any BODY, as well as
+FORM, so you can write recursive and mutually recursive function
+definitions.  See info node `(cl) Function Bindings' for details.
 
 \(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
   (declare (indent 1) (debug cl-flet))
diff --git a/lisp/emacs-lisp/cl.el b/lisp/emacs-lisp/cl.el
index d53c8e0bbc..f6643158d2 100644
--- a/lisp/emacs-lisp/cl.el
+++ b/lisp/emacs-lisp/cl.el
@@ -466,9 +466,12 @@ labels
 	(push var sets)
 	(push (cons (car binding)
                     `(lambda (&rest cl-labels-args)
-                       (cl-list* 'funcall ',var
-                                 cl-labels-args)))
+                       (if (eq (car cl-labels-args) cl--labels-magic)
+                           (list cl--labels-magic ',var)
+                         (cl-list* 'funcall ',var cl-labels-args))))
               newenv)))
+    ;; `lexical-let' adds `cl--function-convert' (which calls
+    ;; `cl--labels-convert') as a macroexpander for `function'.
     (macroexpand-all `(lexical-let ,vars (setq ,@sets) ,@body) newenv)))
 
 ;; Generalized variables are provided by gv.el, but some details are
diff --git a/test/lisp/emacs-lisp/cl-tests.el b/test/lisp/emacs-lisp/cl-tests.el
new file mode 100644
index 0000000000..b673822cd9
--- /dev/null
+++ b/test/lisp/emacs-lisp/cl-tests.el
@@ -0,0 +1,35 @@
+;;; cl-tests.el --- tests for emacs-lisp/cl.el  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2018 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; This program is free software: you can redistribute it and/or
+;; modify it under the terms of the GNU General Public License as
+;; published by the Free Software Foundation, either version 3 of the
+;; License, or (at your option) any later version.
+;;
+;; This program is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see `https://www.gnu.org/licenses/'.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'cl)
+(require 'ert)
+
+\f
+
+(ert-deftest labels-function-quoting ()
+  "Test that #'foo does the right thing in `labels'." ; Bug#31792.
+  (should (eq (funcall (labels ((foo () t))
+                         #'foo))
+              t)))
+
+;;; cl-tests.el ends here
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#31792: 27.0.50; Regression in #'labels, recent versions
  2018-06-14  3:24       ` Noam Postavsky
@ 2018-06-14  3:32         ` Stefan Monnier
  2018-06-14  4:13           ` Noam Postavsky
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2018-06-14  3:32 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Aidan Kehoe, 31792, Alan Mackenzie

>  (defmacro cl-flet (bindings &rest body)
>    "Make local function definitions.
> -Like `cl-labels' but the definitions are not recursive.
> +Bind the function cells of symbols within FORM.

I think this is wrong: it makes it sound like it does `fset`, which
is what CL's `flet` did, but not what `cl-flet` does.
If you look at the implementation, you'll see that it doesn't touch any
"function cell".

> +The bindings only take effect within FORM, not BODY, so you can't
> +write recursive function definitions.  Use `cl-labels' for that.
> +See info node `(cl) Function Bindings' for details.

This is good, thanks.

>  (defmacro cl-labels (bindings &rest body)
> -  "Make temporary function bindings.
> -The bindings can be recursive and the scoping is lexical, but capturing them
> -in closures will only work if `lexical-binding' is in use.
> +    "Make local function definitions.
> +Bind the function cells of symbols within FORM.

Same here about "function cells".

> diff --git a/lisp/emacs-lisp/cl.el b/lisp/emacs-lisp/cl.el
> index d53c8e0bbc..f6643158d2 100644
> --- a/lisp/emacs-lisp/cl.el
> +++ b/lisp/emacs-lisp/cl.el
> @@ -466,9 +466,12 @@ labels
>  	(push var sets)
>  	(push (cons (car binding)
>                      `(lambda (&rest cl-labels-args)
> -                       (cl-list* 'funcall ',var
> -                                 cl-labels-args)))
> +                       (if (eq (car cl-labels-args) cl--labels-magic)
> +                           (list cl--labels-magic ',var)
> +                         (cl-list* 'funcall ',var cl-labels-args))))
>                newenv)))
> +    ;; `lexical-let' adds `cl--function-convert' (which calls
> +    ;; `cl--labels-convert') as a macroexpander for `function'.
>      (macroexpand-all `(lexical-let ,vars (setq ,@sets) ,@body) newenv)))

Good, thanks.


        Stefan





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#31792: 27.0.50; Regression in #'labels, recent versions
  2018-06-14  3:32         ` Stefan Monnier
@ 2018-06-14  4:13           ` Noam Postavsky
  2018-06-14 18:05             ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Noam Postavsky @ 2018-06-14  4:13 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Aidan Kehoe, 31792, Alan Mackenzie

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

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> +Bind the function cells of symbols within FORM.
>
> I think this is wrong: it makes it sound like it does `fset`, which
> is what CL's `flet` did, but not what `cl-flet` does.
> If you look at the implementation, you'll see that it doesn't touch any
> "function cell".

Hmm, the manual page says something similar:

 -- Macro: cl-flet (bindings...) forms...
     This form establishes `let'-style bindings on the function cells
     of symbols rather than on the value cells.

It also wrongly claims that (quote FUNC) would work:

     A "reference" to a function name is either a call to that
     function, or a use of its name quoted by `quote' or `function' to
                                              ^^^^^^^
     be passed on to, say, `mapcar'.

Perhaps both of those are leftovers from the original `flet'
description.  How about this:


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 7072 bytes --]

From f83d79ca2707f4bbe0bc1b0a74ee78d7cc64cbe5 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Tue, 12 Jun 2018 18:41:46 -0400
Subject: [PATCH v3] Fix #'fun handling inside `labels' (Bug#31792)

* lisp/emacs-lisp/cl.el (labels): Apply the equivalent of the
cl-labels change from 2015-01-16 "* lisp/emacs-lisp/cl-macs.el: Fix
last change".
* test/lisp/emacs-lisp/cl-tests.el (labels-function-quoting): New
test.
* lisp/emacs-lisp/cl-macs.el (cl-flet, cl-labels): Improve docstring,
link to relevant manual page.
* doc/misc/cl.texi (Function Bindings): Don't imply that function
cells of symbols are modified by cl-flet.  Don't claim that cl-flet or
cl-labels affect references of the form (quote FUNC).
---
 doc/misc/cl.texi                 | 23 ++++++++++-------------
 lisp/emacs-lisp/cl-macs.el       | 17 ++++++++++++-----
 lisp/emacs-lisp/cl.el            |  7 +++++--
 test/lisp/emacs-lisp/cl-tests.el | 35 +++++++++++++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 20 deletions(-)
 create mode 100644 test/lisp/emacs-lisp/cl-tests.el

diff --git a/doc/misc/cl.texi b/doc/misc/cl.texi
index bf85b00e93..553b935b1e 100644
--- a/doc/misc/cl.texi
+++ b/doc/misc/cl.texi
@@ -1299,17 +1299,18 @@ Function Bindings
 of variables.
 
 @defmac cl-flet (bindings@dots{}) forms@dots{}
-This form establishes @code{let}-style bindings on the function
-cells of symbols rather than on the value cells.  Each @var{binding}
-must be a list of the form @samp{(@var{name} @var{arglist}
-@var{forms}@dots{})}, which defines a function exactly as if
-it were a @code{cl-defun} form.  The function @var{name} is defined
-accordingly but only within the body of the @code{cl-flet}, hiding any external
-definition if applicable.
+This form establishes @code{let}-style bindings for functions rather
+than values.  Each @var{binding} must be a list of the form
+@samp{(@var{name} @var{arglist} @var{body}@dots{})}.  Within
+@var{forms}, any reference to the function @var{name} uses the local
+definition instead of the global one.
+
+A ``reference'' to a function name is either a call to that function,
+or a use of its name quoted by @code{function} to be passed on to,
+say, @code{mapcar}.
 
 The bindings are lexical in scope.  This means that all references to
-the named functions must appear physically within the body of the
-@code{cl-flet} form.
+the named functions must appear physically within @var{forms}.
 
 Functions defined by @code{cl-flet} may use the full Common Lisp
 argument notation supported by @code{cl-defun}; also, the function
@@ -1336,10 +1337,6 @@ Function Bindings
 the functions themselves.  Thus, @code{cl-labels} can define
 local recursive functions, or mutually-recursive sets of functions.
 
-A ``reference'' to a function name is either a call to that
-function, or a use of its name quoted by @code{quote} or
-@code{function} to be passed on to, say, @code{mapcar}.
-
 Note that the @file{cl.el} version of this macro behaves slightly
 differently.  @xref{Obsolete Macros}.
 @end defmac
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 9c47ceae18..0854e665b9 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -1964,13 +1964,16 @@ cl--labels-convert
 ;;;###autoload
 (defmacro cl-flet (bindings &rest body)
   "Make local function definitions.
-Like `cl-labels' but the definitions are not recursive.
-Each binding can take the form (FUNC EXP) where
+Each definition can take the form (FUNC EXP) where
 FUNC is the function name, and EXP is an expression that returns the
 function value to which it should be bound, or it can take the more common
 form \(FUNC ARGLIST BODY...) which is a shorthand
 for (FUNC (lambda ARGLIST BODY)).
 
+FUNC is defined only within FORM, not BODY, so you can't write
+recursive function definitions.  Use `cl-labels' for that.  See
+info node `(cl) Function Bindings' for details.
+
 \(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
   (declare (indent 1) (debug ((&rest (cl-defun)) cl-declarations body)))
   (let ((binds ()) (newenv macroexpand-all-environment))
@@ -2012,9 +2015,13 @@ cl-flet*
 
 ;;;###autoload
 (defmacro cl-labels (bindings &rest body)
-  "Make temporary function bindings.
-The bindings can be recursive and the scoping is lexical, but capturing them
-in closures will only work if `lexical-binding' is in use.
+    "Make local (recursive) function definitions.
+Each definition can take the form (FUNC ARGLIST BODY...) where
+FUNC is the function name, ARGLIST its arguments, and BODY the
+forms of the function body.  FUNC is defined in any BODY, as well
+as FORM, so you can write recursive and mutually recursive
+function definitions.  See info node `(cl) Function Bindings' for
+details.
 
 \(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
   (declare (indent 1) (debug cl-flet))
diff --git a/lisp/emacs-lisp/cl.el b/lisp/emacs-lisp/cl.el
index d53c8e0bbc..f6643158d2 100644
--- a/lisp/emacs-lisp/cl.el
+++ b/lisp/emacs-lisp/cl.el
@@ -466,9 +466,12 @@ labels
 	(push var sets)
 	(push (cons (car binding)
                     `(lambda (&rest cl-labels-args)
-                       (cl-list* 'funcall ',var
-                                 cl-labels-args)))
+                       (if (eq (car cl-labels-args) cl--labels-magic)
+                           (list cl--labels-magic ',var)
+                         (cl-list* 'funcall ',var cl-labels-args))))
               newenv)))
+    ;; `lexical-let' adds `cl--function-convert' (which calls
+    ;; `cl--labels-convert') as a macroexpander for `function'.
     (macroexpand-all `(lexical-let ,vars (setq ,@sets) ,@body) newenv)))
 
 ;; Generalized variables are provided by gv.el, but some details are
diff --git a/test/lisp/emacs-lisp/cl-tests.el b/test/lisp/emacs-lisp/cl-tests.el
new file mode 100644
index 0000000000..b673822cd9
--- /dev/null
+++ b/test/lisp/emacs-lisp/cl-tests.el
@@ -0,0 +1,35 @@
+;;; cl-tests.el --- tests for emacs-lisp/cl.el  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2018 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; This program is free software: you can redistribute it and/or
+;; modify it under the terms of the GNU General Public License as
+;; published by the Free Software Foundation, either version 3 of the
+;; License, or (at your option) any later version.
+;;
+;; This program is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see `https://www.gnu.org/licenses/'.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'cl)
+(require 'ert)
+
+\f
+
+(ert-deftest labels-function-quoting ()
+  "Test that #'foo does the right thing in `labels'." ; Bug#31792.
+  (should (eq (funcall (labels ((foo () t))
+                         #'foo))
+              t)))
+
+;;; cl-tests.el ends here
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#31792: 27.0.50; Regression in #'labels, recent versions
  2018-06-14  4:13           ` Noam Postavsky
@ 2018-06-14 18:05             ` Stefan Monnier
  2018-06-20  0:08               ` Noam Postavsky
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2018-06-14 18:05 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Aidan Kehoe, 31792, Alan Mackenzie

>> I think this is wrong: it makes it sound like it does `fset`, which
>> is what CL's `flet` did, but not what `cl-flet` does.
>> If you look at the implementation, you'll see that it doesn't touch any
>> "function cell".
>
> Hmm, the manual page says something similar:
>
>  -- Macro: cl-flet (bindings...) forms...
>      This form establishes `let'-style bindings on the function cells
>      of symbols rather than on the value cells.
>
> It also wrongly claims that (quote FUNC) would work:
>
>      A "reference" to a function name is either a call to that
>      function, or a use of its name quoted by `quote' or `function' to
>                                               ^^^^^^^
>      be passed on to, say, `mapcar'.
>
> Perhaps both of those are leftovers from the original `flet'
> description.

Sounds like it, yes.

> How about this:

Looks good to me, thank you,


        Stefan





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#31792: 27.0.50; Regression in #'labels, recent versions
  2018-06-14 18:05             ` Stefan Monnier
@ 2018-06-20  0:08               ` Noam Postavsky
  0 siblings, 0 replies; 11+ messages in thread
From: Noam Postavsky @ 2018-06-20  0:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Aidan Kehoe, 31792, Alan Mackenzie

tags 31792 fixed
close 31792 26.2
quit

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Looks good to me, thank you,

Pushed to emacs-26.

[1: e292c0973c]: 2018-06-19 20:02:16 -0400
  Fix #'fun handling inside `labels' (Bug#31792)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e292c0973cf7a92819d312ea8a828b67e6adf1ab





^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-06-20  0:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-11 23:11 bug#31792: 27.0.50; Regression in #'labels, recent versions Aidan Kehoe
2018-06-11 23:31 ` Noam Postavsky
2018-06-12 22:58   ` Noam Postavsky
2018-06-13 13:16     ` Stefan Monnier
2018-06-14  3:24       ` Noam Postavsky
2018-06-14  3:32         ` Stefan Monnier
2018-06-14  4:13           ` Noam Postavsky
2018-06-14 18:05             ` Stefan Monnier
2018-06-20  0:08               ` Noam Postavsky
     [not found] ` <mailman.1699.1528759928.1292.bug-gnu-emacs@gnu.org>
2018-06-13 16:57   ` Alan Mackenzie
2018-06-13 17:03     ` Noam Postavsky

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).