GNU bug report logs - #25280
25.1; define-inline doesn't support &rest

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 25280 in the body.
You can then email your comments to 25280 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: Leo Liu <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1; define-inline doesn't support &rest
Date: Tue, 27 Dec 2016 10:42:13 +0800

inline.el has a comment FIXME: How can this work with CL arglists? but
it is worse. it doesn't support &rest.

Try compile the following example:

  (define-inline rest (&rest xs)
    (inline-quote (apply #'vector ,xs)))
  
  (princ (rest 1 2))

In toplevel form:
test.el:7:1:Warning: ‘1’ is a malformed function

The header comment says defsubst: not as efficient. Could this be made
clearer? In what way is defsubst less efficient?

What is the outlook for defsubst or cl-defsubst? Are they on their way
out? 

Thanks,
Leo

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 25280 <at> debbugs.gnu.org
Subject: Re: bug#25280: 25.1; define-inline doesn't support &rest
Date: Mon, 26 Dec 2016 22:21:13 -0500

> inline.el has a comment FIXME: How can this work with CL arglists? but
> it is worse. it doesn't support &rest.

It does, but IIRC you need to use inline-letevals to evaluate the args (it
is usually needed for most/all args).

>   (define-inline rest (&rest xs)
>     (inline-quote (apply #'vector ,xs)))

   (define-inline rest (&rest xs)
     (inline-leteval xs
       (inline-quote (apply #'vector ',xs))))


-- Stefan

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 25280 <at> debbugs.gnu.org
Subject: Re: bug#25280: 25.1; define-inline doesn't support &rest
Date: Tue, 27 Dec 2016 12:25:03 +0800

On 2016-12-26 22:21 -0500, Stefan Monnier wrote:
>    (define-inline rest (&rest xs)
>      (inline-leteval xs
>        (inline-quote (apply #'vector ',xs))))

Thanks. It gets me further but still problems. New example:

;;; -*- lexical-binding: t -*-

(define-inline rest (&rest xs)
  (inline-letevals xs (inline-quote (apply #'vector ',xs))))

(princ (rest 1 2))                            ; A
(let ((x 1) (y 2)) (princ (rest x y)))        ; B

So A prints [1 2] and B [x y] i.e. x y is not eval'd. thoughts?

Leo

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 25280 <at> debbugs.gnu.org
Subject: Re: bug#25280: 25.1; define-inline doesn't support &rest
Date: Tue, 27 Dec 2016 10:25:25 -0500

> (let ((x 1) (y 2)) (princ (rest x y)))        ; B
> So A prints [1 2] and B [x y] i.e. x y is not eval'd. thoughts?

Hmm... indeed... and it breaks down even with (rest (+ x 1) y).


        Stefan

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 25280 <at> debbugs.gnu.org
Subject: Re: bug#25280: 25.1; define-inline doesn't support &rest
Date: Tue, 27 Dec 2016 12:42:44 -0500

>> (let ((x 1) (y 2)) (princ (rest x y)))        ; B
>> So A prints [1 2] and B [x y] i.e. x y is not eval'd. thoughts?

> Hmm... indeed... and it breaks down even with (rest (+ x 1) y).

Wait, I was confused: `xs` is a list of expressions, so of course ', is
not the right way to place it in there.  To turn it into an expression
that evaluates to a list of values, you generally want (list . ,xs):

    (define-inline sm-foo (&rest xs)
      (inline-letevals xs (inline-quote (apply #'vector (list . ,xs)))))

and I think this works correctly.
OTOH it's not as efficient as we'd like.  The better way to write it is:

    (define-inline sm-foo (&rest xs)
      (inline-letevals xs (inline-quote (vector . ,xs))))

but I now see that this is not handled properly in the case the function
is not inlined.  More specifically, the `sm-foo` function gets defined as:

    (lambda (&rest xs) (apply vector xs))

where the `vector` failed to be quoted with #'.
I installed the patch below into `master` which should fix it.


        Stefan


diff --git a/lisp/emacs-lisp/inline.el b/lisp/emacs-lisp/inline.el
index 058c56c3b49..5ceb0d9ed29 100644
--- a/lisp/emacs-lisp/inline.el
+++ b/lisp/emacs-lisp/inline.el
@@ -191,9 +191,9 @@ After VARS is handled, BODY is evaluated in the new environment."
        (while (and (consp exp) (not (eq '\, (car exp))))
          (push (inline--dont-quote (pop exp)) args))
        (setq args (nreverse args))
-       (if exp
-           `(apply ,@args ,(inline--dont-quote exp))
-         args)))
+       (if (null exp)
+           args
+         `(apply #',(car args) ,@(cdr args) ,(inline--dont-quote exp)))))
     (_ exp)))
 
 (defun inline--do-leteval (var-exp &rest body)

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 25280 <at> debbugs.gnu.org
Subject: Re: bug#25280: 25.1; define-inline doesn't support &rest
Date: Thu, 29 Dec 2016 10:00:11 +0800

On 2016-12-27 12:42 -0500, Stefan Monnier wrote:
> OTOH it's not as efficient as we'd like.  The better way to write it is:
>
>     (define-inline sm-foo (&rest xs)
>       (inline-letevals xs (inline-quote (vector . ,xs))))

I misunderstood no support for `,@' implied `. ,'. Good to know it is
not the case. My experiment seems to suggest that inline-letevals is
only needed for variables that are eval'd more than once. Is that
understanding correct?

I also get compiler warning: 

In rest:
t2.el:4:38:Warning: reference to free variable ‘vector’

where t2.el has the contents:

;;; -*- lexical-binding: t -*-

(define-inline rest (&rest xs)
  (inline-letevals xs (inline-quote (vector . ,xs))))

Thanks,
Leo

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 25280 <at> debbugs.gnu.org
Subject: Re: bug#25280: 25.1; define-inline doesn't support &rest
Date: Thu, 29 Dec 2016 10:41:53 +0800

On 2016-12-29 10:00 +0800, Leo Liu wrote:
> In rest:
> t2.el:4:38:Warning: reference to free variable ‘vector’

Ok, this is the bug that is fixed in commit
e6161f648903d821865b9610b3b6aa0f82a5dcb7.

Leo

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 25280 <at> debbugs.gnu.org
Subject: Re: bug#25280: 25.1; define-inline doesn't support &rest
Date: Wed, 28 Dec 2016 22:25:40 -0500

> I misunderstood no support for `,@' implied `. ,'.

No: I can turn

    (a b c . ,d)

into

    (apply #'a b c d)

but doing that for

    (a b ,@c d)

is more cumbersome.

> Good to know it is not the case.  My experiment seems to suggest that
> inline-letevals is only needed for variables that are eval'd more than
> once.

There are cases where inline-letevals can be skipped, indeed, but
"eval'd only once" is not quite sufficient: you also have to make sure
it's eval'd at least once, and that the various arguments are evaluated
in the right order and before anything else happens (to stay true to
the behavior of a function call).

> I also get compiler warning: 
>
> In rest:
> t2.el:4:38:Warning: reference to free variable ‘vector’
>
> where t2.el has the contents:
>
> ;;; -*- lexical-binding: t -*-
>
> (define-inline rest (&rest xs)
>   (inline-letevals xs (inline-quote (vector . ,xs))))

I assume this is with an Emacs build that doesn't yet have my recent
patch, right?


        Stefan

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 25280-done <at> debbugs.gnu.org
Subject: Re: bug#25280: 25.1; define-inline doesn't support &rest
Date: Thu, 29 Dec 2016 13:54:05 +0800

version: 25.2

On 2016-12-28 22:25 -0500, Stefan Monnier wrote:
>> I misunderstood no support for `,@' implied `. ,'.
>
> No: I can turn
>
>     (a b c . ,d)
>
> into
>
>     (apply #'a b c d)
>
> but doing that for
>
>     (a b ,@c d)
>
> is more cumbersome.

Thanks.

>> Good to know it is not the case.  My experiment seems to suggest that
>> inline-letevals is only needed for variables that are eval'd more than
>> once.
>
> There are cases where inline-letevals can be skipped, indeed, but
> "eval'd only once" is not quite sufficient: you also have to make sure
> it's eval'd at least once, and that the various arguments are evaluated
> in the right order and before anything else happens (to stay true to
> the behavior of a function call).

Understood.

>> In rest:
>> t2.el:4:38:Warning: reference to free variable ‘vector’
> I assume this is with an Emacs build that doesn't yet have my recent
> patch, right?

Exactly.

Leo

Previous Next

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