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