all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Andrea Corallo <akrl@sdf.org>
To: Alan Mackenzie <acm@muc.de>
Cc: 58739@debbugs.gnu.org
Subject: bug#58739: Lack of error message about number of args (?native compilation?)
Date: Mon, 31 Oct 2022 13:15:18 +0000	[thread overview]
Message-ID: <xjfcza8f82h.fsf@ma.sdf.org> (raw)
In-Reply-To: <Y10W1Antoh5Mue7A@ACM> (Alan Mackenzie's message of "Sat, 29 Oct 2022 12:04:36 +0000")

Alan Mackenzie <acm@muc.de> writes:

> Hello, Andrea.
>
> On Tue, Oct 25, 2022 at 20:15:38 +0000, Andrea Corallo wrote:
>> Alan Mackenzie <acm@muc.de> writes:
>
>> > On Sun, Oct 23, 2022 at 12:12:49 +0000, Alan Mackenzie wrote:
>
> [ .... ]
>
>> >> 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).
>
> We'll see what Eli says.
>
>> I think also that throwing the error in 'declare_imported_func' is okay
>> at this point.
>
> Apologies at this point.  I should have produced an error from this
> before bothering you.  I was unable to produce such an error, and I've
> spent the last few days understanding what happens here, with this
> result:
>
> A call from a native compiled function is always in the form nargs +
> *args, regardless of whether there are more than 8 arguments or not.
> More accurately, the call to an unknown type of function (.eln/.elc/.el)
> puts the function (in some form, I don't know exactly what) at element 0
> of *args, and the arguments themselves starting at element 1 of *args.
> It then calls Ffuncall (or something like it).
>
> When the called function is a byte-code function, further down the call
> stack exec_byte_code gets called with this (nargs + 1) + *args.
> exec_byte_code then pushes the arguments onto the stack before
> interpreting the byte code.
>
> So this "problem" with the native compiler not knowing what call sequence
> to generate isn't a problem at all.  It's all dealt with at run time.  No
> doubt this slows down Emacs quite a bit, but it's safe.
>
> So I'll take that bit out of my patch, and commit the rest of it to
> master, and see what happens from there.

Hi Alan,

That's correct.  As you prefer, we could also keep it as assertion.

Thanks

  Andrea





  parent reply	other threads:[~2022-10-31 13:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-23 12:12 bug#58739: Lack of error message about number of args (?native compilation?) Alan Mackenzie
2022-10-24 16:41 ` Alan Mackenzie
2022-10-25 20:15   ` Andrea Corallo
2022-10-29 12:04     ` Alan Mackenzie
2022-10-29 12:48       ` Eli Zaretskii
2022-10-29 16:04         ` Alan Mackenzie
2022-10-31 13:15       ` Andrea Corallo [this message]
     [not found] ` <handler.58739.B.166652719021045.ack@debbugs.gnu.org>
2022-10-29 13:32   ` bug#58739: Fixed Alan Mackenzie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xjfcza8f82h.fsf@ma.sdf.org \
    --to=akrl@sdf.org \
    --cc=58739@debbugs.gnu.org \
    --cc=acm@muc.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.