[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#58739: Lack of error message about number of args (?native compilati
From: |
Andrea Corallo |
Subject: |
bug#58739: Lack of error message about number of args (?native compilation?) |
Date: |
Tue, 25 Oct 2022 20:15:38 +0000 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) |
Alan Mackenzie <acm@muc.de> writes:
> Hello, Andrea.
>
> On Sun, Oct 23, 2022 at 12:12:49 +0000, Alan Mackenzie wrote:
>> Hello, Emacs.
>
>> Firstly, note that the function desktop-buffer has exactly 12
>> parameters.
>
>> With an up to date Emacs 29:
>
>> (i) emacs -Q
>> (ii) M-x load-library RET desktop RET.
>> (iii) M-x disassemble RET desktop-buffer.
>
>> Note that this is native code.
>
>> (iv) M-: (desktop-buffer 1 2 3 4 5 6 7 8 9 10 11 12 13) RET
>
>> This gives an error message about 4 not being a list. What it ought to
>> do is instead throw an error about the number of arguments. This is a
>> bug.
>
>> (v) M-x load-file RET /path/to/emacs/lisp/desktop.elc.
>> (vi) M-x disassemble RET desktop-buffer.
>
>> Note that we now have byte compiled code.
>
>> (vii) M-: (desktop-buffer 1 2 3 4 5 6 7 8 9 10 11 12 13) RET
>
>> We now get a correct error message about the numbere of arguments.
>
>> As a matter of interest, I noticed this bug while byte-compiling
>> desktop.el inside Emacs. It gave a warning message about the number of
>> parameters to desktop-buffer having changed from 12+ to 12.
>
>> Here, I suspect there's a bug in the native compilation of
>> desktop-buffer.
>
> The problem here is that (func-arity 'desktop-buffer) returns (12 . 12) on a
> byte compiled desktop.elc, but (12 . many) on the corresponding .eln file.
> This (12 . many) must be regarded as a bug, since there are no &rest
> parameters in desktop-buffer.
>
> I propose a minor amendment to the definition of MANY, such that it will
> mean "there are &rest parameters", rather than "the calling convention
> is with nargs + *args". I have implemented this, and my patch is below.
>
> What I want to ask you to check is that in the native compiler, when
> declare_imported_func encounters a function with, say, exactly 12
> arguments, it will throw an error. I think this is actually correct,
> since the compiler cannot know whether this function uses the subr
> calling convention of nargs + *args, or the byte coded convention of
> pushing the 12 arguments individually onto the stack. Is throwing this
> error a good idea?
Hi Alan,
to me this fix looks like a good idea (assuming changing the definition
of MANY is acceptable).
I think also that throwing the error in 'declare_imported_func' is okay
at this point.
Just two small nits (forgive me please :) :
> Thanks!
>
> Here's my proposed patch:
[...]
> }
> + else if (nargs > 8)
> + /* The calling convention for a function with a fixed number of
> + arguments greater than eight is different between a native
^^^
I think this should be aligned with the initial 'The'
> + compiled function and a byte compiled function. Thus we
> + cannot safely compile it with the native compiler. */
> + signal_error ("Imported function has too many arguments safely to
> compile natively",
I think we should break this string to stay within 78 characters (see
CONTRIBUTE).
> + subr_sym);
> else if (!types)
> {
> types = SAFE_ALLOCA (nargs * sizeof (* types));
> diff --git a/src/eval.c b/src/eval.c
> index 8810136c04..495dbb53b5 100644
> --- a/src/eval.c
> +++ b/src/eval.c
> @@ -2433,7 +2433,9 @@ eval_sub (Lisp_Object form)
>
> else if (XSUBR (fun)->max_args == UNEVALLED)
> val = (XSUBR (fun)->function.aUNEVALLED) (args_left);
> - else if (XSUBR (fun)->max_args == MANY)
> + else if (XSUBR (fun)->max_args == MANY
> + || XSUBR (fun)->max_args > 8)
> +
> {
> /* Pass a vector of evaluated arguments. */
> Lisp_Object *vals;
> @@ -2996,7 +2998,8 @@ funcall_subr (struct Lisp_Subr *subr, ptrdiff_t
> numargs, Lisp_Object *args)
Likewise
> if (numargs >= subr->min_args)
> {
> /* Conforming call to finite-arity subr. */
> - if (numargs <= subr->max_args)
> + if (numargs <= subr->max_args
> + && subr->max_args <= 8)
> {
> Lisp_Object argbuf[8];
> Lisp_Object *a;
> @@ -3032,15 +3035,13 @@ funcall_subr (struct Lisp_Subr *subr, ptrdiff_t
> numargs, Lisp_Object *args)
Likewise
> return subr->function.a8 (a[0], a[1], a[2], a[3], a[4], a[5],
> a[6], a[7]);
[...]
Thanks for the patch!
Andrea
Message not available