[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#64642: 29.0.92; Native compiler doesn't compile dynamically bound fu
From: |
Andrea Corallo |
Subject: |
bug#64642: 29.0.92; Native compiler doesn't compile dynamically bound functions. |
Date: |
Mon, 17 Jul 2023 09:42:31 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Alan Mackenzie <acm@muc.de> writes:
> Hello, Eli and Andrea.
>
> On Sat, Jul 15, 2023 at 12:53:51 +0300, Eli Zaretskii wrote:
>> > Date: Sat, 15 Jul 2023 09:38:13 +0000
>> > From: Alan Mackenzie <acm@muc.de>
>
>> > Hello, Emacs.
>
>> > In the Emacs 29 pretest version (or the master version):
>> > (i) emacs -Q
>> > (ii) C-x b foo.el <RET>
>> > (iii) M-x emacs-lisp-mode <RET>
>> > Don't set lexical-binding in this buffer.
>> > (iv) Enter a function called foo:
>> > (defun foo () "foo doc string"
>> > (lambda (bar) "lambda doc string" (car bar)))
>> > (v) With point after the function, evaluate it with C-x C-e.
>
>> > (vi) M-: (native-compile 'foo)
>> > This signals an error, native-compiler-error-dyn-func. This is a bug.
>
>> > #########################################################################
>
>> > The immediate cause of the bug is in the version of
>> > comp-spill-lap-function which processes named functions (comp.el).
>> > Unlike the other version of the cl-defmethod (which processes lambda
>> > functions), there is no code for dynamic functions here.
>
>> > If the intention is not to process dynamic functions, this should be
>> > indicated by an error message rather than a signal. Personally, I feel
>> > that dynamic functions ought to be handled in Emacs-29.
>
>> > Fixing this bug should be relatively straightforward, since it should
>> > only involve copying and adapting the corresponding code in the lambda
>> > version of comp-spill-lap-function.
>
>> Adding Andrea.
>
>> It is unlikely that this will be fixed in Emacs 29, unless the fix is
>> so simple that will surprise me.
>
> Here is a fix. Its simplicity probably doesn't surprise you (Eli), but
> the change is entirely within one function, and a lot of the patch is
> just whitespace changes.
>
> There are no problems running make bootstrap or make check with patch in
> place. It also solves the bug; unless there are good reasons dynamically
> bound functions weren't handled. Andrea?
Hi Alan,
the fix LGTM thanks for the patch, I think we only need to add a test as
well to exercise this code, something like the "free-fun" test in
comp-tests.el.
Thanks
Andrea