Re: master 2a7f01aa38: Fix initialization of module functions

Re: master 2a7f01aa38: Fix initialization of module functions

[Stefan Monnier]

>  allocate_module_function (void)
>  {
>    return ALLOCATE_PSEUDOVECTOR (struct Lisp_Module_Function,
> -                                interactive_form, PVEC_MODULE_FUNCTION);
> +                                command_modes, PVEC_MODULE_FUNCTION);
>  }
>  
>  #define XSET_MODULE_FUNCTION(var, ptr) \

Usually we accompany that with a comment in the struct stating "this
is the last GC-tracked field" or something like that.


        Stefan

Re: master 2a7f01aa38: Fix initialization of module functions

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Thu, 10 Nov 2022 09:06:35 -0500
> 
> >  allocate_module_function (void)
> >  {
> >    return ALLOCATE_PSEUDOVECTOR (struct Lisp_Module_Function,
> > -                                interactive_form, PVEC_MODULE_FUNCTION);
> > +                                command_modes, PVEC_MODULE_FUNCTION);
> >  }
> >  
> >  #define XSET_MODULE_FUNCTION(var, ptr) \
> 
> Usually we accompany that with a comment in the struct stating "this
> is the last GC-tracked field" or something like that.

There's already such a comment, a few lines above.

Re: master 2a7f01aa38: Fix initialization of module functions

[Stefan Monnier]

Eli Zaretskii [2022-11-10 17:23:52] wrote:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: Eli Zaretskii <eliz@gnu.org>
>> Date: Thu, 10 Nov 2022 09:06:35 -0500
>> 
>> >  allocate_module_function (void)
>> >  {
>> >    return ALLOCATE_PSEUDOVECTOR (struct Lisp_Module_Function,
>> > -                                interactive_form, PVEC_MODULE_FUNCTION);
>> > +                                command_modes, PVEC_MODULE_FUNCTION);
>> >  }
>> >  
>> >  #define XSET_MODULE_FUNCTION(var, ptr) \
>> 
>> Usually we accompany that with a comment in the struct stating "this
>> is the last GC-tracked field" or something like that.
>
> There's already such a comment, a few lines above.

The comment doesn't say which one is the last tracked field.


        Stefan

Re: master 2a7f01aa38: Fix initialization of module functions

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Thu, 10 Nov 2022 12:44:56 -0500
> 
> Eli Zaretskii [2022-11-10 17:23:52] wrote:
> 
> >> >  allocate_module_function (void)
> >> >  {
> >> >    return ALLOCATE_PSEUDOVECTOR (struct Lisp_Module_Function,
> >> > -                                interactive_form, PVEC_MODULE_FUNCTION);
> >> > +                                command_modes, PVEC_MODULE_FUNCTION);
> >> >  }
> >> >  
> >> >  #define XSET_MODULE_FUNCTION(var, ptr) \
> >> 
> >> Usually we accompany that with a comment in the struct stating "this
> >> is the last GC-tracked field" or something like that.
> >
> > There's already such a comment, a few lines above.
> 
> The comment doesn't say which one is the last tracked field.

??? That's basic C, or what am I missing?

Anyway, feel free to add whatever comment you want, you don't need to
convince me (in this case).

Re: master 2a7f01aa38: Fix initialization of module functions

[Stefan Monnier]

>> >> >  allocate_module_function (void)
>> >> >  {
>> >> >    return ALLOCATE_PSEUDOVECTOR (struct Lisp_Module_Function,
>> >> > -                                interactive_form, PVEC_MODULE_FUNCTION);
>> >> > +                                command_modes, PVEC_MODULE_FUNCTION);
>> >> >  }
>> >> >  
>> >> >  #define XSET_MODULE_FUNCTION(var, ptr) \
>> >> 
>> >> Usually we accompany that with a comment in the struct stating "this
>> >> is the last GC-tracked field" or something like that.
>> >
>> > There's already such a comment, a few lines above.
>> 
>> The comment doesn't say which one is the last tracked field.
>
> ??? That's basic C, or what am I missing?

The commit 2a7f01aa38 fixes a bug introduced by commit 58e0c8ee86e2
which did (among other things:)

    @@ -548,7 +548,7 @@
     {
       union vectorlike_header header;
     
       /* Fields traced by GC; these must come first.  */
    -  Lisp_Object documentation, interactive_form;
    +  Lisp_Object documentation, interactive_form, command_modes;
     
       /* Fields ignored by GC.  */

so clearly the comment was not sufficiently clear for Lars to notice
that adding a field after `interactive_form` would introduce a bug.


        Stefan