bug#25280: 25.1; define-inline doesn't support &rest

bug#25280: 25.1; define-inline doesn't support &rest

[Leo Liu]

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

bug#25280: 25.1; define-inline doesn't support &rest

[Stefan Monnier]

> 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

bug#25280: 25.1; define-inline doesn't support &rest

[Leo Liu]

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

bug#25280: 25.1; define-inline doesn't support &rest

[Stefan Monnier]

> (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

bug#25280: 25.1; define-inline doesn't support &rest

[Stefan Monnier]

>> (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)

bug#25280: 25.1; define-inline doesn't support &rest

[Leo Liu]

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

bug#25280: 25.1; define-inline doesn't support &rest

[Leo Liu]

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

bug#25280: 25.1; define-inline doesn't support &rest

[Stefan Monnier]

> 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

bug#25280: 25.1; define-inline doesn't support &rest

[Leo Liu]

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