GNU bug report logs - #78887
feature/igc [PATCH] Trace struct kboard exactly

Previous Next

To reply to this bug, email your comments to 78887 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Helmut Eller <eller.helmut <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: feature/igc [PATCH] Trace struct kboard exactly
Date: Tue, 24 Jun 2025 08:26:47 +0200

[Message part 1 (text/plain, inline)]

This is a proposal to trace struct kboard exactly.  It comes
in two parts:
  1. trace the kbd_macro_buffer exactly
  2. trace the other fields of the kboard struct exactly.

[0001-Trace-kboard.kbd_macro_buffer-exactly.patch (text/x-diff, attachment)]

[0002-Trace-kboards-as-exact-roots.patch (text/x-diff, attachment)]

Message #8 received at 78887 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: 78887 <at> debbugs.gnu.org
Subject: Re: bug#78887: feature/igc [PATCH] Trace struct kboard exactly
Date: Tue, 24 Jun 2025 12:22:07 +0000

"Helmut Eller" <eller.helmut <at> gmail.com> writes:

> This is a proposal to trace struct kboard exactly.  It comes
> in two parts:
>   1. trace the kbd_macro_buffer exactly
>   2. trace the other fields of the kboard struct exactly.

This looks good.  It'd be nice to turn these into properly MPS-allocated
objects at some point, but it'd require additional changes elsewhere.

> +static mps_res_t
> +scan_kboard (mps_ss_t ss, void *start, void *, void *)
> +{

Emacs doesn't usually omit parameter names of unused parameters;
technically, it's a C23 feature and we don't require compilers to be
that new.  (IMHO, it's also much less readable, and it does break
existing tools).

> +struct kboard *
> +igc_xalloc_kboard_exact (void)

Why not just "igc_alloc_kboard"?

> Lisp_Object *
> igc_xnrealloc_lisp_objs_exact (ptrdiff_t nitems_old,
> 			       Lisp_Object old[nitems_old],
> 			       ptrdiff_t nitems_new,
> 			       const char *label)

Are we sure that declaration won't make clang assume old isn't NULL when
nitems_old == 0?  The behavior of zero-length arrays has changed a lot
in C recently, and I'm not sure whether all versions we strive to
support understand that Lisp_Object old[nitems_old] is a plain pointer,
which might be NULL.

In any case, I'd prefer to use the old-style "ptrdiff_t nitems_old,
Lisp_Object *old", for now, and to change the Emacs coding style for all
of Emacs (including it's many MANY DEFUNs) if and when we decide to.

Thanks!
Pip

Message #11 received at 78887 <at> debbugs.gnu.org (full text, mbox):

From: Helmut Eller <eller.helmut <at> gmail.com>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 78887 <at> debbugs.gnu.org
Subject: Re: bug#78887: feature/igc [PATCH] Trace struct kboard exactly
Date: Wed, 25 Jun 2025 10:42:59 +0200

[Message part 1 (text/plain, inline)]

On Tue, Jun 24 2025, Pip Cet wrote:

> "Helmut Eller" <eller.helmut <at> gmail.com> writes:
[...]
>> +static mps_res_t
>> +scan_kboard (mps_ss_t ss, void *start, void *, void *)
>> +{
>
> Emacs doesn't usually omit parameter names of unused parameters;
> technically, it's a C23 feature and we don't require compilers to be
> that new.  (IMHO, it's also much less readable, and it does break
> existing tools).

Ok. I changed that.

>> +struct kboard *
>> +igc_xalloc_kboard_exact (void)
>
> Why not just "igc_alloc_kboard"?

I was emulating the pattern established by igc_xalloc_lisp_objs_exact.

>> Lisp_Object *
>> igc_xnrealloc_lisp_objs_exact (ptrdiff_t nitems_old,
>> 			       Lisp_Object old[nitems_old],
>> 			       ptrdiff_t nitems_new,
>> 			       const char *label)
>
> Are we sure that declaration won't make clang assume old isn't NULL when
> nitems_old == 0?  The behavior of zero-length arrays has changed a lot
> in C recently, and I'm not sure whether all versions we strive to
> support understand that Lisp_Object old[nitems_old] is a plain pointer,
> which might be NULL.
>
> In any case, I'd prefer to use the old-style "ptrdiff_t nitems_old,
> Lisp_Object *old", for now, and to change the Emacs coding style for all
> of Emacs (including it's many MANY DEFUNs) if and when we decide to.

Since C99,

  int main (int argc, char *argv[argc])

is the same as

  int main (int argc, char *argv[*])

or for that matter

  int main (int argc, char *argv[argc - 1])

I would be surprised if my code would have caused any problems; I changed
it anyway:

[0001-Trace-kboard.kbd_macro_buffer-exactly.patch (text/x-diff, attachment)]

[0002-Trace-kboards-as-exact-roots.patch (text/x-diff, attachment)]

Message #14 received at 78887 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: 78887 <at> debbugs.gnu.org
Subject: Re: bug#78887: feature/igc [PATCH] Trace struct kboard exactly
Date: Wed, 25 Jun 2025 09:11:11 +0000

"Helmut Eller" <eller.helmut <at> gmail.com> writes:

> On Tue, Jun 24 2025, Pip Cet wrote:
>
>> "Helmut Eller" <eller.helmut <at> gmail.com> writes:
> [...]
>>> +static mps_res_t
>>> +scan_kboard (mps_ss_t ss, void *start, void *, void *)
>>> +{
>>
>> Emacs doesn't usually omit parameter names of unused parameters;
>> technically, it's a C23 feature and we don't require compilers to be
>> that new.  (IMHO, it's also much less readable, and it does break
>> existing tools).
>
> Ok. I changed that.

Thanks!  Applied, with one additional change to remove "foo".

>>> Lisp_Object *
>>> igc_xnrealloc_lisp_objs_exact (ptrdiff_t nitems_old,
>>> 			       Lisp_Object old[nitems_old],
>>> 			       ptrdiff_t nitems_new,
>>> 			       const char *label)
>>
>> Are we sure that declaration won't make clang assume old isn't NULL when
>> nitems_old == 0?  The behavior of zero-length arrays has changed a lot
>> in C recently, and I'm not sure whether all versions we strive to
>> support understand that Lisp_Object old[nitems_old] is a plain pointer,
>> which might be NULL.
>>
>> In any case, I'd prefer to use the old-style "ptrdiff_t nitems_old,
>> Lisp_Object *old", for now, and to change the Emacs coding style for all
>> of Emacs (including it's many MANY DEFUNs) if and when we decide to.
>
> Since C99,
>
>   int main (int argc, char *argv[argc])
>
> is the same as
>
>   int main (int argc, char *argv[*])
>
> or for that matter
>
>   int main (int argc, char *argv[argc - 1])

So it's the same as

int main (int argc, char **argv)

too, I assume.

> I would be surprised if my code would have caused any problems; I changed
> it anyway:

I was surprised by the zero-byte memcpy "optimization" that broke the
last commit, so I figured I'd ask about this one.  TBH, if it's a C99
feature and all it does is make the array size explicit without implying
that the pointer is valid if the element count is zero, we should use it
more generally.

> [2. text/x-diff; 0001-Trace-kboard.kbd_macro_buffer-exactly.patch]...
>
> [3. text/x-diff; 0002-Trace-kboards-as-exact-roots.patch]...

> +static int _Noreturn foo(void)
> +{
> +  return 1;
> +}
> +

I've removed that function definition.

Thanks again!

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.