./configure --enable-check-lisp-object-type

./configure --enable-check-lisp-object-type

[Lars Ingebrigtsen]

For the first time in a while I enabled the Lisp object type, and I was
given a wall of

dispnew.c:6593:1: note: in expansion of macro ‘DEFUN’
 6593 | DEFUN ("internal-show-cursor-p", Finternal_show_cursor_p,
      | ^~~~~
lisp.h:3179:5: warning: missing braces around initializer [-Wmissing-braces]
 3179 |     {{{{ PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },                         \
      |     ^

for every DEFUN.  That's new, isn't it?  (This is on Debian/bookworm.)

This is from this:

#define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc)	\
  SUBR_SECTION_ATTRIBUTE                                                \
  static union Aligned_Lisp_Subr sname =                                \
     {{{ PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },				\
       { .a ## maxargs = fnname },					\
       minargs, maxargs, lname, {intspec}, 0}};				\
   Lisp_Object fnname

That seems like beaucoup de braces, so is that a gcc bug or something?

(Adding more braces makes compilation fail.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: ./configure --enable-check-lisp-object-type

[Stefan Monnier]

> For the first time in a while I enabled the Lisp object type, and I was
> given a wall of

I've been using it for many many years.

> dispnew.c:6593:1: note: in expansion of macro ‘DEFUN’
>  6593 | DEFUN ("internal-show-cursor-p", Finternal_show_cursor_p,
>       | ^~~~~
> lisp.h:3179:5: warning: missing braces around initializer [-Wmissing-braces]
>  3179 |     {{{{ PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },                         \
>       |     ^
>
> for every DEFUN.  That's new, isn't it?  (This is on Debian/bookworm.)

Yes, it's new enough that I haven't reported it yet.

> This is from this:
>
> #define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc)	\
>   SUBR_SECTION_ATTRIBUTE                                                \
>   static union Aligned_Lisp_Subr sname =                                \
>      {{{ PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },				\
>        { .a ## maxargs = fnname },					\
>        minargs, maxargs, lname, {intspec}, 0}};				\
>    Lisp_Object fnname
>
> That seems like beaucoup de braces, so is that a gcc bug or something?
> (Adding more braces makes compilation fail.)

I don't know the syntax of C initializers well enough to judge, but
I wasn't able to silence the warning by tweaking the code, so far.


        Stefan

Re: ./configure --enable-check-lisp-object-type

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> Date: Tue, 12 Apr 2022 13:23:21 -0400
> 
> > For the first time in a while I enabled the Lisp object type, and I was
> > given a wall of
> 
> I've been using it for many many years.
> 
> > dispnew.c:6593:1: note: in expansion of macro ‘DEFUN’
> >  6593 | DEFUN ("internal-show-cursor-p", Finternal_show_cursor_p,
> >       | ^~~~~
> > lisp.h:3179:5: warning: missing braces around initializer [-Wmissing-braces]
> >  3179 |     {{{{ PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },                         \
> >       |     ^
> >
> > for every DEFUN.  That's new, isn't it?  (This is on Debian/bookworm.)
> 
> Yes, it's new enough that I haven't reported it yet.

The last change there was done 2 years ago, if my Git forensics are
correct.  So I wonder what exactly happened recently that started
triggering this.

> > #define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc)	\
> >   SUBR_SECTION_ATTRIBUTE                                                \
> >   static union Aligned_Lisp_Subr sname =                                \
> >      {{{ PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },				\
> >        { .a ## maxargs = fnname },					\
> >        minargs, maxargs, lname, {intspec}, 0}};				\
> >    Lisp_Object fnname
> >
> > That seems like beaucoup de braces, so is that a gcc bug or something?
> > (Adding more braces makes compilation fail.)
> 
> I don't know the syntax of C initializers well enough to judge, but
> I wasn't able to silence the warning by tweaking the code, so far.

I believe the problem is this:

  static union Aligned_Lisp_Subr sname =                                \
     {{{ PVEC_SUBR << PSEUDOVECTOR_AREA_BITS },				\
       { .a ## maxargs = fnname },					\
       minargs, maxargs, lname, {intspec}, 0}};				\
                                           ^

That zero corresponds to the 'Lisp_Object command_modes' member of
Aligned_Lisp_Subr, and in a build with --enable-check-lisp-object-type
a Lisp_Object is a struct, so it must be initialized with {0}, not
just a scalar zero.  This is what we had before April 2020, when
Andrea removed the braces around the zero in commit d73e6407.

Andrea, can you tell why this was done?  I don't think I understand
how the intent of that changeset justifies the removal.

Re: ./configure --enable-check-lisp-object-type

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> That zero corresponds to the 'Lisp_Object command_modes' member of
> Aligned_Lisp_Subr, and in a build with --enable-check-lisp-object-type
> a Lisp_Object is a struct, so it must be initialized with {0}, not
> just a scalar zero.

I can confirm that adding braces around the zero makes these warnings go
away.

There's similar warnings in alloc:

alloc.c: In function 'syms_of_alloc':
alloc.c:7838:6: warning: missing braces around initializer [-Wmissing-braces]
 7838 |      {{{ PSEUDOVECTOR_FLAG | (PVEC_SUBR << PSEUDOVECTOR_AREA_BITS) },
      |      ^
 7839 |        { .a4 = watch_gc_cons_threshold },
 7840 |        4, 4, "watch_gc_cons_threshold", {0}, 0}};
      |                                              {}

And adding braces around the zero does the trick there, too.

There's still a number of other warnings which may or may not be valid
(I haven't actually looked at them):

In file included from dbusbind.c:26:
dbusbind.c: In function 'xd_signature':
lisp.h:409:38: warning: potential null pointer dereference [-Wnull-dereference]
  409 | #define lisp_h_XCAR(c) XCONS (c)->u.s.car
      |                        ~~~~~~~~~~~~~~^~~~
lisp.h:1498:10: note: in expansion of macro 'lisp_h_XCAR'
 1498 |   return lisp_h_XCAR (c);
      |          ^~~~~~~~~~~
lisp.h:409:38: warning: potential null pointer dereference [-Wnull-dereference]
  409 | #define lisp_h_XCAR(c) XCONS (c)->u.s.car
      |                        ~~~~~~~~~~~~~~^~~~

And a some other warnings, too -- about half a dozen, I'd say.

Is the --enable-check-lisp-object-type build supposed to be without
warnings, or has that never been the ambition?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: ./configure --enable-check-lisp-object-type

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  Andrea Corallo
>  <akrl@sdf.org>,  emacs-devel@gnu.org
> Date: Tue, 12 Apr 2022 20:27:42 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > That zero corresponds to the 'Lisp_Object command_modes' member of
> > Aligned_Lisp_Subr, and in a build with --enable-check-lisp-object-type
> > a Lisp_Object is a struct, so it must be initialized with {0}, not
> > just a scalar zero.
> 
> I can confirm that adding braces around the zero makes these warnings go
> away.
> 
> There's similar warnings in alloc:
> 
> alloc.c: In function 'syms_of_alloc':
> alloc.c:7838:6: warning: missing braces around initializer [-Wmissing-braces]
>  7838 |      {{{ PSEUDOVECTOR_FLAG | (PVEC_SUBR << PSEUDOVECTOR_AREA_BITS) },
>       |      ^
>  7839 |        { .a4 = watch_gc_cons_threshold },
>  7840 |        4, 4, "watch_gc_cons_threshold", {0}, 0}};
>       |                                              {}
> 
> And adding braces around the zero does the trick there, too.

And if you build without --enable-check-lisp-object-type, does it
compile cleanly with the additional braces?

> There's still a number of other warnings which may or may not be valid
> (I haven't actually looked at them):
> 
> In file included from dbusbind.c:26:
> dbusbind.c: In function 'xd_signature':
> lisp.h:409:38: warning: potential null pointer dereference [-Wnull-dereference]
>   409 | #define lisp_h_XCAR(c) XCONS (c)->u.s.car
>       |                        ~~~~~~~~~~~~~~^~~~
> lisp.h:1498:10: note: in expansion of macro 'lisp_h_XCAR'
>  1498 |   return lisp_h_XCAR (c);
>       |          ^~~~~~~~~~~
> lisp.h:409:38: warning: potential null pointer dereference [-Wnull-dereference]
>   409 | #define lisp_h_XCAR(c) XCONS (c)->u.s.car
>       |                        ~~~~~~~~~~~~~~^~~~

I don't think this is related, it might be that defining Lisp_Object
as a struct makes it easier (or harder) for the compiler to reason
about the control flow.

> Is the --enable-check-lisp-object-type build supposed to be without
> warnings, or has that never been the ambition?

It's supposed to be as clean as without that option, yes.  It just
seems that this option is rarely used, so it has bitrotted to some
extent.

Re: ./configure --enable-check-lisp-object-type

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> And if you build without --enable-check-lisp-object-type, does it
> compile cleanly with the additional braces?

Ah, no:

alloc.c:7847:8: warning: braces around scalar initializer
 7847 |        4, 4, "watch_gc_cons_percentage", {0}, {0}}};
      |        ^


And similarly, the braces in DEFUN leads to:

xmenu.c:2773:1: note: in expansion of macro 'DEFUN'
 2773 | DEFUN ("menu-or-popup-active-p", Fmenu_or_popup_active_p, Smenu_or_popup_active_p, 0, 0, 0,
      | ^~~~~
lisp.h:3178:16: note: (near initialization for 'Smenu_or_popup_active_p.s.command_modes')

in a non-enable-check build.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: ./configure --enable-check-lisp-object-type

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  akrl@sdf.org,  emacs-devel@gnu.org
> Date: Tue, 12 Apr 2022 20:40:31 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > And if you build without --enable-check-lisp-object-type, does it
> > compile cleanly with the additional braces?
> 
> Ah, no:
> 
> alloc.c:7847:8: warning: braces around scalar initializer
>  7847 |        4, 4, "watch_gc_cons_percentage", {0}, {0}}};
>       |        ^
> 
> 
> And similarly, the braces in DEFUN leads to:
> 
> xmenu.c:2773:1: note: in expansion of macro 'DEFUN'
>  2773 | DEFUN ("menu-or-popup-active-p", Fmenu_or_popup_active_p, Smenu_or_popup_active_p, 0, 0, 0,
>       | ^~~~~
> lisp.h:3178:16: note: (near initialization for 'Smenu_or_popup_active_p.s.command_modes')
> 
> in a non-enable-check build.

Which means we need 2 different initializations, conditioned on
CHECK_LISP_OBJECT_TYPE.

Re: ./configure --enable-check-lisp-object-type

[Stefan Monnier]

> Which means we need 2 different initializations, conditioned on
> CHECK_LISP_OBJECT_TYPE.

We can probably

    #ifdef CHECK_LISP_OBJECT_TYPE
    # define LISP_NIL_INITIALIZER {0}
    #else
    # define LISP_NIL_INITIALIZER 0
    #endif


-- Stefan

Re: ./configure --enable-check-lisp-object-type

[martin rudalics]

 > It just
 > seems that this option is rarely used, so it has bitrotted to some
 > extent.

I'm using it always but I rarely pull master.  FWIW builds in February
did not show the warnings.  Nowadays I build with -Wno-missing-braces
only.

martin