all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#25280: 25.1; define-inline doesn't support &rest
@ 2016-12-27  2:42 Leo Liu
  2016-12-27  3:21 ` Stefan Monnier
  0 siblings, 1 reply; 9+ messages in thread
From: Leo Liu @ 2016-12-27  2:42 UTC (permalink / raw)
  To: 25280; +Cc: Stefan Monnier


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





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

* bug#25280: 25.1; define-inline doesn't support &rest
  2016-12-27  2:42 bug#25280: 25.1; define-inline doesn't support &rest Leo Liu
@ 2016-12-27  3:21 ` Stefan Monnier
  2016-12-27  4:25   ` Leo Liu
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2016-12-27  3:21 UTC (permalink / raw)
  To: Leo Liu; +Cc: 25280

> 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





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

* bug#25280: 25.1; define-inline doesn't support &rest
  2016-12-27  3:21 ` Stefan Monnier
@ 2016-12-27  4:25   ` Leo Liu
  2016-12-27 15:25     ` Stefan Monnier
  0 siblings, 1 reply; 9+ messages in thread
From: Leo Liu @ 2016-12-27  4:25 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25280

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





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

* bug#25280: 25.1; define-inline doesn't support &rest
  2016-12-27  4:25   ` Leo Liu
@ 2016-12-27 15:25     ` Stefan Monnier
  2016-12-27 17:42       ` Stefan Monnier
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2016-12-27 15:25 UTC (permalink / raw)
  To: Leo Liu; +Cc: 25280

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





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

* bug#25280: 25.1; define-inline doesn't support &rest
  2016-12-27 15:25     ` Stefan Monnier
@ 2016-12-27 17:42       ` Stefan Monnier
  2016-12-29  2:00         ` Leo Liu
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2016-12-27 17:42 UTC (permalink / raw)
  To: Leo Liu; +Cc: 25280

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





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

* bug#25280: 25.1; define-inline doesn't support &rest
  2016-12-27 17:42       ` Stefan Monnier
@ 2016-12-29  2:00         ` Leo Liu
  2016-12-29  2:41           ` Leo Liu
  2016-12-29  3:25           ` Stefan Monnier
  0 siblings, 2 replies; 9+ messages in thread
From: Leo Liu @ 2016-12-29  2:00 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25280

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





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

* bug#25280: 25.1; define-inline doesn't support &rest
  2016-12-29  2:00         ` Leo Liu
@ 2016-12-29  2:41           ` Leo Liu
  2016-12-29  3:25           ` Stefan Monnier
  1 sibling, 0 replies; 9+ messages in thread
From: Leo Liu @ 2016-12-29  2:41 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25280

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





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

* bug#25280: 25.1; define-inline doesn't support &rest
  2016-12-29  2:00         ` Leo Liu
  2016-12-29  2:41           ` Leo Liu
@ 2016-12-29  3:25           ` Stefan Monnier
  2016-12-29  5:54             ` Leo Liu
  1 sibling, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2016-12-29  3:25 UTC (permalink / raw)
  To: Leo Liu; +Cc: 25280

> 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





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

* bug#25280: 25.1; define-inline doesn't support &rest
  2016-12-29  3:25           ` Stefan Monnier
@ 2016-12-29  5:54             ` Leo Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Leo Liu @ 2016-12-29  5:54 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25280-done

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





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

end of thread, other threads:[~2016-12-29  5:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-27  2:42 bug#25280: 25.1; define-inline doesn't support &rest Leo Liu
2016-12-27  3:21 ` Stefan Monnier
2016-12-27  4:25   ` Leo Liu
2016-12-27 15:25     ` Stefan Monnier
2016-12-27 17:42       ` Stefan Monnier
2016-12-29  2:00         ` Leo Liu
2016-12-29  2:41           ` Leo Liu
2016-12-29  3:25           ` Stefan Monnier
2016-12-29  5:54             ` Leo Liu

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.