bug#64646: Master: Native compiler doesn't always compile lambda forms.

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Alan Mackenzie]

In the master branch:

(i) emacs -Q
(ii) C-x b foo.el <RET>
(iii) Insert into foo.el:
    ;; -*- lexical-binding:t -*-
(iv) M-x emacs-lisp-mode
(v) Insert into foo.el:
    (defun foo () "foo doc string"
      (lambda (bar) "lambda doc string" (car bar)))
(vi) With point after the function, C-x C-e to evaluate it.

(vii) M-: (native-compile 'foo)
This returns #<subr foo>
(viii) M-: (foo)
This returns the lambda form as a byte-compiled function.  This is a bug:
it should return the lambda form as a native-compiled function.

Note: this bug is also in the emacs-29 branch.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Eli Zaretskii]

> Date: Sat, 15 Jul 2023 12:10:06 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
> In the master branch:
> 
> (i) emacs -Q
> (ii) C-x b foo.el <RET>
> (iii) Insert into foo.el:
>     ;; -*- lexical-binding:t -*-
> (iv) M-x emacs-lisp-mode
> (v) Insert into foo.el:
>     (defun foo () "foo doc string"
>       (lambda (bar) "lambda doc string" (car bar)))
> (vi) With point after the function, C-x C-e to evaluate it.
> 
> (vii) M-: (native-compile 'foo)
> This returns #<subr foo>
> (viii) M-: (foo)
> This returns the lambda form as a byte-compiled function.  This is a bug:
> it should return the lambda form as a native-compiled function.

Why do you think it should return the native-compiled form?  Based on
what?

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Alan Mackenzie]

Hello, Eli.

On Sat, Jul 15, 2023 at 16:06:18 +0300, Eli Zaretskii wrote:
> > Date: Sat, 15 Jul 2023 12:10:06 +0000
> > From: Alan Mackenzie <acm@muc.de>

> > In the master branch:

> > (i) emacs -Q
> > (ii) C-x b foo.el <RET>
> > (iii) Insert into foo.el:
> >     ;; -*- lexical-binding:t -*-
> > (iv) M-x emacs-lisp-mode
> > (v) Insert into foo.el:
> >     (defun foo () "foo doc string"
> >       (lambda (bar) "lambda doc string" (car bar)))
> > (vi) With point after the function, C-x C-e to evaluate it.

> > (vii) M-: (native-compile 'foo)
> > This returns #<subr foo>
> > (viii) M-: (foo)
> > This returns the lambda form as a byte-compiled function.  This is a bug:
> > it should return the lambda form as a native-compiled function.

> Why do you think it should return the native-compiled form?  Based on
> what?

Well, if the native compilation compiled _all_ of the defun, the lambda
form would also have been compiled, and surely that is what should have
been returned.  There is some suspicion that the native compilation on
this defun is incomplete.

When we do byte-compilation, the returned value is byte-compiled.  By
analogy, when we do NC, the return value should be NC'd.

I can't see any reason to return a byte-compiled function.  The fact that
we use byte-compilation as a part of native-compilation should be an
internal detail, not visible from outside.

Also, the returned lambda function being byte-compiled will cause it to
run more slowly that if it were native-compiled.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Michael Heerdegen]

Alan Mackenzie <acm@muc.de> writes:

> (vii) M-: (native-compile 'foo)
> This returns #<subr foo>
> (viii) M-: (foo)

I apologize when this is a dumb question, but: is this call using the
native-compiled function?

Michael.

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Alan Mackenzie]

Hello, Michael.

On Sun, Jul 16, 2023 at 06:16:13 +0200, Michael Heerdegen wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > (vii) M-: (native-compile 'foo)
> > This returns #<subr foo>
> > (viii) M-: (foo)

> I apologize when this is a dumb question, but: is this call using the
> native-compiled function?

It's not at all a dumb question, it's very pertinent.

After step (vii), M-: (symbol-function 'foo) <RET> returns

    #<subr foo>

..  So I think the call in (viii) is indeed using the native-compiled
function.

> Michael.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Michael Heerdegen]

Alan Mackenzie <acm@muc.de> writes:

> After step (vii), M-: (symbol-function 'foo) <RET> returns
>
>     #<subr foo>
>
> ..  So I think the call in (viii) is indeed using the native-compiled
> function.

Thanks.  Yes, then I would also expect it to return the created function
as a native-compiled function object.

Michael.

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Andrea Corallo]

Alan Mackenzie <acm@muc.de> writes:

> In the master branch:
>
> (i) emacs -Q
> (ii) C-x b foo.el <RET>
> (iii) Insert into foo.el:
>     ;; -*- lexical-binding:t -*-
> (iv) M-x emacs-lisp-mode
> (v) Insert into foo.el:
>     (defun foo () "foo doc string"
>       (lambda (bar) "lambda doc string" (car bar)))
> (vi) With point after the function, C-x C-e to evaluate it.
>
> (vii) M-: (native-compile 'foo)
> This returns #<subr foo>
> (viii) M-: (foo)
> This returns the lambda form as a byte-compiled function.  This is a bug:
> it should return the lambda form as a native-compiled function.
>
> Note: this bug is also in the emacs-29 branch.

Hi Alan,

I can reproduce, (native-compile 'foo) compiles only foo, compiling the
whole compilation unit with eg `emacs-lisp-native-compile-and-load'
compiles as expected also the inner lambda.

I'm not 100% convinced this behaviour is a bug tho.

  Andrea

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Alan Mackenzie]

Hello, Andrea.

On Mon, Jul 17, 2023 at 09:17:13 -0400, Andrea Corallo wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > In the master branch:

> > (i) emacs -Q
> > (ii) C-x b foo.el <RET>
> > (iii) Insert into foo.el:
> >     ;; -*- lexical-binding:t -*-
> > (iv) M-x emacs-lisp-mode
> > (v) Insert into foo.el:
> >     (defun foo () "foo doc string"
> >       (lambda (bar) "lambda doc string" (car bar)))
> > (vi) With point after the function, C-x C-e to evaluate it.

> > (vii) M-: (native-compile 'foo)
> > This returns #<subr foo>
> > (viii) M-: (foo)
> > This returns the lambda form as a byte-compiled function.  This is a bug:
> > it should return the lambda form as a native-compiled function.

> > Note: this bug is also in the emacs-29 branch.

> Hi Alan,

> I can reproduce, (native-compile 'foo) compiles only foo, compiling the
> whole compilation unit with eg `emacs-lisp-native-compile-and-load'
> compiles as expected also the inner lambda.

Why would compiling a .el file compile inner lambda forms, but
native-compile doesn't?

> I'm not 100% convinced this behaviour is a bug tho.

I don't understand that.  Why might it be incorrect to compile that inner
lambda natively?

>   Andrea

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Andrea Corallo]

Alan Mackenzie <acm@muc.de> writes:

> Hello, Andrea.
>
> On Mon, Jul 17, 2023 at 09:17:13 -0400, Andrea Corallo wrote:
>> Alan Mackenzie <acm@muc.de> writes:
>
>> > In the master branch:
>
>> > (i) emacs -Q
>> > (ii) C-x b foo.el <RET>
>> > (iii) Insert into foo.el:
>> >     ;; -*- lexical-binding:t -*-
>> > (iv) M-x emacs-lisp-mode
>> > (v) Insert into foo.el:
>> >     (defun foo () "foo doc string"
>> >       (lambda (bar) "lambda doc string" (car bar)))
>> > (vi) With point after the function, C-x C-e to evaluate it.
>
>> > (vii) M-: (native-compile 'foo)
>> > This returns #<subr foo>
>> > (viii) M-: (foo)
>> > This returns the lambda form as a byte-compiled function.  This is a bug:
>> > it should return the lambda form as a native-compiled function.
>
>> > Note: this bug is also in the emacs-29 branch.
>
>> Hi Alan,
>
>> I can reproduce, (native-compile 'foo) compiles only foo, compiling the
>> whole compilation unit with eg `emacs-lisp-native-compile-and-load'
>> compiles as expected also the inner lambda.
>
> Why would compiling a .el file compile inner lambda forms, but
> native-compile doesn't?
>
>> I'm not 100% convinced this behaviour is a bug tho.
>
> I don't understand that.  Why might it be incorrect to compile that inner
> lambda natively?

Hi Alan,

I'm not saying it would be incorrect.  I'm suggesting that if is not
specified what's the expected behaviour of compiling by name the outer
lambda it might not be a bug.

When we compile a whole compilation unit we indeed have to compile all
functions, in this case what we promised is I think not defined.

  Andrea

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Alan Mackenzie]

Hello, Andrea.

This bug doesn't seem to be moving, so ....

On Wed, Jul 26, 2023 at 10:57:01 -0400, Andrea Corallo wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > Hello, Andrea.

> > On Mon, Jul 17, 2023 at 09:17:13 -0400, Andrea Corallo wrote:
> >> Alan Mackenzie <acm@muc.de> writes:

> >> > In the master branch:

> >> > (i) emacs -Q
> >> > (ii) C-x b foo.el <RET>
> >> > (iii) Insert into foo.el:
> >> >     ;; -*- lexical-binding:t -*-
> >> > (iv) M-x emacs-lisp-mode
> >> > (v) Insert into foo.el:
> >> >     (defun foo () "foo doc string"
> >> >       (lambda (bar) "lambda doc string" (car bar)))
> >> > (vi) With point after the function, C-x C-e to evaluate it.

> >> > (vii) M-: (native-compile 'foo)
> >> > This returns #<subr foo>
> >> > (viii) M-: (foo)
> >> > This returns the lambda form as a byte-compiled function.  This is a bug:
> >> > it should return the lambda form as a native-compiled function.

> >> > Note: this bug is also in the emacs-29 branch.

> >> Hi Alan,

> >> I can reproduce, (native-compile 'foo) compiles only foo, compiling the
> >> whole compilation unit with eg `emacs-lisp-native-compile-and-load'
> >> compiles as expected also the inner lambda.

> > Why would compiling a .el file compile inner lambda forms, but
> > native-compile doesn't?

> >> I'm not 100% convinced this behaviour is a bug tho.

> > I don't understand that.  Why might it be incorrect to compile that inner
> > lambda natively?

> Hi Alan,

> I'm not saying it would be incorrect.  I'm suggesting that if is not
> specified what's the expected behaviour of compiling by name the outer
> lambda it might not be a bug.

> When we compile a whole compilation unit we indeed have to compile all
> functions, in this case what we promised is I think not defined.

I still don't understand that.  The doc string for native-compile says:

    Compile FUNCTION-OR-FILE into native code.

..  I can't see any reason not also to compile inner lambda functions
natively.

Anyhow, to fix this bug (if such it be) is easy:


diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index 181e5ca96a1..2360fbaa494 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -1359,7 +1359,12 @@ comp-add-func-to-ctxt
               (comp-ctxt-top-level-forms comp-ctxt)
               (list (make-byte-to-native-func-def :name function-name
                                                   :c-name c-name)))
-        (comp-add-func-to-ctxt func))))
+        (comp-add-func-to-ctxt func))
+      ;; Handle any lambda functions in BYTE-CODE.
+      (maphash (lambda (key val)
+                 (unless (eq key (aref byte-code 1))
+                   (comp-intern-func-in-ctxt key val)))
+               byte-to-native-lambdas-h)))

 (cl-defmethod comp-spill-lap-function ((form list))
   "Byte-compile FORM, spilling data from the byte compiler."


What do you say?

Incidentally, the code in the various comp-spill-lap-function methods
together with comp-intern-func-in-ctxt appears to have some code
duplication.  Would it be possible to have the symbol and list methods of
comp-spill-lap-function simply call comp-intern-func-in-ctxt the way the
string method does?  That would simplify those two methods quite a bit.

>   Andrea

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Andrea Corallo]

Alan Mackenzie <acm@muc.de> writes:

> Hello, Andrea.
>
> This bug doesn't seem to be moving, so ....
>
> On Wed, Jul 26, 2023 at 10:57:01 -0400, Andrea Corallo wrote:
>> Alan Mackenzie <acm@muc.de> writes:
>
>> > Hello, Andrea.
>
>> > On Mon, Jul 17, 2023 at 09:17:13 -0400, Andrea Corallo wrote:
>> >> Alan Mackenzie <acm@muc.de> writes:
>
>> >> > In the master branch:
>
>> >> > (i) emacs -Q
>> >> > (ii) C-x b foo.el <RET>
>> >> > (iii) Insert into foo.el:
>> >> >     ;; -*- lexical-binding:t -*-
>> >> > (iv) M-x emacs-lisp-mode
>> >> > (v) Insert into foo.el:
>> >> >     (defun foo () "foo doc string"
>> >> >       (lambda (bar) "lambda doc string" (car bar)))
>> >> > (vi) With point after the function, C-x C-e to evaluate it.
>
>> >> > (vii) M-: (native-compile 'foo)
>> >> > This returns #<subr foo>
>> >> > (viii) M-: (foo)
>> >> > This returns the lambda form as a byte-compiled function.  This is a bug:
>> >> > it should return the lambda form as a native-compiled function.
>
>> >> > Note: this bug is also in the emacs-29 branch.
>
>> >> Hi Alan,
>
>> >> I can reproduce, (native-compile 'foo) compiles only foo, compiling the
>> >> whole compilation unit with eg `emacs-lisp-native-compile-and-load'
>> >> compiles as expected also the inner lambda.
>
>> > Why would compiling a .el file compile inner lambda forms, but
>> > native-compile doesn't?
>
>> >> I'm not 100% convinced this behaviour is a bug tho.
>
>> > I don't understand that.  Why might it be incorrect to compile that inner
>> > lambda natively?
>
>> Hi Alan,
>
>> I'm not saying it would be incorrect.  I'm suggesting that if is not
>> specified what's the expected behaviour of compiling by name the outer
>> lambda it might not be a bug.
>
>> When we compile a whole compilation unit we indeed have to compile all
>> functions, in this case what we promised is I think not defined.
>
> I still don't understand that.  The doc string for native-compile says:
>
>     Compile FUNCTION-OR-FILE into native code.
>
> ..  I can't see any reason not also to compile inner lambda functions
> natively.
>
> Anyhow, to fix this bug (if such it be) is easy:
>
> diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
> index 181e5ca96a1..2360fbaa494 100644
> --- a/lisp/emacs-lisp/comp.el
> +++ b/lisp/emacs-lisp/comp.el
> @@ -1359,7 +1359,12 @@ comp-add-func-to-ctxt
>                (comp-ctxt-top-level-forms comp-ctxt)
>                (list (make-byte-to-native-func-def :name function-name
>                                                    :c-name c-name)))
> -        (comp-add-func-to-ctxt func))))
> +        (comp-add-func-to-ctxt func))
> +      ;; Handle any lambda functions in BYTE-CODE.
> +      (maphash (lambda (key val)
> +                 (unless (eq key (aref byte-code 1))
> +                   (comp-intern-func-in-ctxt key val)))
> +               byte-to-native-lambdas-h)))
>
>  (cl-defmethod comp-spill-lap-function ((form list))
>    "Byte-compile FORM, spilling data from the byte compiler."
>
>
> What do you say?

LGTM as long as indeed it does not regress any test. Speaking of which
with the patch I guess we want a test to cover this.

> Incidentally, the code in the various comp-spill-lap-function methods
> together with comp-intern-func-in-ctxt appears to have some code
> duplication.  Would it be possible to have the symbol and list methods of
> comp-spill-lap-function simply call comp-intern-func-in-ctxt the way the
> string method does?  That would simplify those two methods quite a bit.

Mmmh maybe, I think one has to try to see if the result is satisfactory.

Thanks

  Andrea

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Alan Mackenzie]

Hello, Andrea.

On Thu, Nov 02, 2023 at 13:32:21 -0400, Andrea Corallo wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > This bug doesn't seem to be moving, so ....

> > On Wed, Jul 26, 2023 at 10:57:01 -0400, Andrea Corallo wrote:
> >> Alan Mackenzie <acm@muc.de> writes:

> >> >> I'm not 100% convinced this behaviour is a bug tho.

> >> > I don't understand that.  Why might it be incorrect to compile that inner
> >> > lambda natively?

> >> Hi Alan,

> >> I'm not saying it would be incorrect.  I'm suggesting that if is not
> >> specified what's the expected behaviour of compiling by name the outer
> >> lambda it might not be a bug.

> >> When we compile a whole compilation unit we indeed have to compile all
> >> functions, in this case what we promised is I think not defined.

> > I still don't understand that.  The doc string for native-compile says:

> >     Compile FUNCTION-OR-FILE into native code.

> > ..  I can't see any reason not also to compile inner lambda functions
> > natively.

> > Anyhow, to fix this bug (if such it be) is easy:

> > diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
> > index 181e5ca96a1..2360fbaa494 100644
> > --- a/lisp/emacs-lisp/comp.el
> > +++ b/lisp/emacs-lisp/comp.el
> > @@ -1359,7 +1359,12 @@ comp-add-func-to-ctxt
> >                (comp-ctxt-top-level-forms comp-ctxt)
> >                (list (make-byte-to-native-func-def :name function-name
> >                                                    :c-name c-name)))
> > -        (comp-add-func-to-ctxt func))))
> > +        (comp-add-func-to-ctxt func))
> > +      ;; Handle any lambda functions in BYTE-CODE.
> > +      (maphash (lambda (key val)
> > +                 (unless (eq key (aref byte-code 1))
> > +                   (comp-intern-func-in-ctxt key val)))
> > +               byte-to-native-lambdas-h)))

> >  (cl-defmethod comp-spill-lap-function ((form list))
> >    "Byte-compile FORM, spilling data from the byte compiler."


> > What do you say?

> LGTM as long as indeed it does not regress any test. Speaking of which
> with the patch I guess we want a test to cover this.

Thanks.  I've committed a patch for this, including two extra tests which
test that a nested lambda function also gets native compiled.

I'm closing the bug with this post.

> > Incidentally, the code in the various comp-spill-lap-function methods
> > together with comp-intern-func-in-ctxt appears to have some code
> > duplication.  Would it be possible to have the symbol and list methods of
> > comp-spill-lap-function simply call comp-intern-func-in-ctxt the way the
> > string method does?  That would simplify those two methods quite a bit.

> Mmmh maybe, I think one has to try to see if the result is satisfactory.

I've done this refactoring too.  The symbol and list methods for
comp-spill-lap-function now have 17 and 15 lines respectively.  I hope
you like it!

> Thanks

>   Andrea

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#64646: Master: Native compiler doesn't always compile lambda forms.

[Andrea Corallo]

Alan Mackenzie <acm@muc.de> writes:

> Hello, Andrea.
>
> On Thu, Nov 02, 2023 at 13:32:21 -0400, Andrea Corallo wrote:
>> Alan Mackenzie <acm@muc.de> writes:
>
>> > This bug doesn't seem to be moving, so ....
>
>> > On Wed, Jul 26, 2023 at 10:57:01 -0400, Andrea Corallo wrote:
>> >> Alan Mackenzie <acm@muc.de> writes:
>
>> >> >> I'm not 100% convinced this behaviour is a bug tho.
>
>> >> > I don't understand that.  Why might it be incorrect to compile that inner
>> >> > lambda natively?
>
>> >> Hi Alan,
>
>> >> I'm not saying it would be incorrect.  I'm suggesting that if is not
>> >> specified what's the expected behaviour of compiling by name the outer
>> >> lambda it might not be a bug.
>
>> >> When we compile a whole compilation unit we indeed have to compile all
>> >> functions, in this case what we promised is I think not defined.
>
>> > I still don't understand that.  The doc string for native-compile says:
>
>> >     Compile FUNCTION-OR-FILE into native code.
>
>> > ..  I can't see any reason not also to compile inner lambda functions
>> > natively.
>
>> > Anyhow, to fix this bug (if such it be) is easy:
>
>> > diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
>> > index 181e5ca96a1..2360fbaa494 100644
>> > --- a/lisp/emacs-lisp/comp.el
>> > +++ b/lisp/emacs-lisp/comp.el
>> > @@ -1359,7 +1359,12 @@ comp-add-func-to-ctxt
>> >                (comp-ctxt-top-level-forms comp-ctxt)
>> >                (list (make-byte-to-native-func-def :name function-name
>> >                                                    :c-name c-name)))
>> > -        (comp-add-func-to-ctxt func))))
>> > +        (comp-add-func-to-ctxt func))
>> > +      ;; Handle any lambda functions in BYTE-CODE.
>> > +      (maphash (lambda (key val)
>> > +                 (unless (eq key (aref byte-code 1))
>> > +                   (comp-intern-func-in-ctxt key val)))
>> > +               byte-to-native-lambdas-h)))
>
>> >  (cl-defmethod comp-spill-lap-function ((form list))
>> >    "Byte-compile FORM, spilling data from the byte compiler."
>
>
>> > What do you say?
>
>> LGTM as long as indeed it does not regress any test. Speaking of which
>> with the patch I guess we want a test to cover this.
>
> Thanks.  I've committed a patch for this, including two extra tests which
> test that a nested lambda function also gets native compiled.
>
> I'm closing the bug with this post.
>
>> > Incidentally, the code in the various comp-spill-lap-function methods
>> > together with comp-intern-func-in-ctxt appears to have some code
>> > duplication.  Would it be possible to have the symbol and list methods of
>> > comp-spill-lap-function simply call comp-intern-func-in-ctxt the way the
>> > string method does?  That would simplify those two methods quite a bit.
>
>> Mmmh maybe, I think one has to try to see if the result is satisfactory.
>
> I've done this refactoring too.  The symbol and list methods for
> comp-spill-lap-function now have 17 and 15 lines respectively.  I hope
> you like it!

Look nice thanks!

  Andrea