Re: master f92520d009e 1/2: Don't pause display for pending input

Re: master f92520d009e 1/2: Don't pause display for pending input

[Andrea Corallo]

Gerd Moellmann <gerd@gnu.org> writes:

> branch: master
> commit f92520d009e1ab8024b03d9d71ecc9820573c040
> Author: Gerd Möllmann <gerd@gnu.org>
> Commit: Gerd Möllmann <gerd@gnu.org>
>
>     Don't pause display for pending input
>
>     * src/dispnew.c: Remove display_completed, redisplay_dont_pause,
>     redisplay-dont-pause was declared obsolete in Emacs 24.  Remove anything
>     checking pending input, change function signatures accordingly, and so
>     on.
>
>     * src/keyboard.c (read_char): Don't use redisplay_dont_pause.
>     * src/minibuf.c (read_minibuf): Use new function signatures.
>     * src/xdisp.c: Don't check display_completed. Use new API.
>
>     * lisp/subr.el (redisplay-dont-pause): Remove declaration.

I think instances of 'Qredisplay_dont_pause' are still present in the
codebase so some build configurations are now broken (at least my
GNU/Linux one).

=====
xterm.c:6372:13: error: ‘Qredisplay_dont_pause’ undeclared (first use in this function)
 6372 |   specbind (Qredisplay_dont_pause, Qt);
      |             ^~~~~~~~~~~~~~~~~~~~~
xterm.c:6372:13: note: each undeclared identifier is reported only once for each function it appears in
xfns.c: In function ‘Fx_print_frames_dialog’:
xfns.c:9967:13: error: ‘Qredisplay_dont_pause’ undeclared (first use in this function)
 9967 |   specbind (Qredisplay_dont_pause, Qt);
      |             ^~~~~~~~~~~~~~~~~~~~~
xfns.c:9967:13: note: each undeclared identifier is reported only once for each function it appears in
=====

Could you have a look?

Thanks

Re: master f92520d009e 1/2: Don't pause display for pending input

[Gerd Möllmann]

Andrea Corallo <acorallo@gnu.org> writes:

> Gerd Moellmann <gerd@gnu.org> writes:
>
>> branch: master
>> commit f92520d009e1ab8024b03d9d71ecc9820573c040
>> Author: Gerd Möllmann <gerd@gnu.org>
>> Commit: Gerd Möllmann <gerd@gnu.org>
>>
>>     Don't pause display for pending input
>>
>>     * src/dispnew.c: Remove display_completed, redisplay_dont_pause,
>>     redisplay-dont-pause was declared obsolete in Emacs 24.  Remove anything
>>     checking pending input, change function signatures accordingly, and so
>>     on.
>>
>>     * src/keyboard.c (read_char): Don't use redisplay_dont_pause.
>>     * src/minibuf.c (read_minibuf): Use new function signatures.
>>     * src/xdisp.c: Don't check display_completed. Use new API.
>>
>>     * lisp/subr.el (redisplay-dont-pause): Remove declaration.
>
> I think instances of 'Qredisplay_dont_pause' are still present in the
> codebase so some build configurations are now broken (at least my
> GNU/Linux one).
>
> =====
> xterm.c:6372:13: error: ‘Qredisplay_dont_pause’ undeclared (first use in this function)
>  6372 |   specbind (Qredisplay_dont_pause, Qt);
>       |             ^~~~~~~~~~~~~~~~~~~~~
> xterm.c:6372:13: note: each undeclared identifier is reported only once for each function it appears in
> xfns.c: In function ‘Fx_print_frames_dialog’:
> xfns.c:9967:13: error: ‘Qredisplay_dont_pause’ undeclared (first use in this function)
>  9967 |   specbind (Qredisplay_dont_pause, Qt);
>       |             ^~~~~~~~~~~~~~~~~~~~~
> xfns.c:9967:13: note: each undeclared identifier is reported only once for each function it appears in
> =====
>
> Could you have a look?
>
> Thanks

Thanks, should be fixed now.

Re: master f92520d009e 1/2: Don't pause display for pending input

[Andrea Corallo]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Andrea Corallo <acorallo@gnu.org> writes:
>
>> Gerd Moellmann <gerd@gnu.org> writes:
>>
>>> branch: master
>>> commit f92520d009e1ab8024b03d9d71ecc9820573c040
>>> Author: Gerd Möllmann <gerd@gnu.org>
>>> Commit: Gerd Möllmann <gerd@gnu.org>
>>>
>>>     Don't pause display for pending input
>>>
>>>     * src/dispnew.c: Remove display_completed, redisplay_dont_pause,
>>>     redisplay-dont-pause was declared obsolete in Emacs 24.  Remove anything
>>>     checking pending input, change function signatures accordingly, and so
>>>     on.
>>>
>>>     * src/keyboard.c (read_char): Don't use redisplay_dont_pause.
>>>     * src/minibuf.c (read_minibuf): Use new function signatures.
>>>     * src/xdisp.c: Don't check display_completed. Use new API.
>>>
>>>     * lisp/subr.el (redisplay-dont-pause): Remove declaration.
>>
>> I think instances of 'Qredisplay_dont_pause' are still present in the
>> codebase so some build configurations are now broken (at least my
>> GNU/Linux one).
>>
>> =====
>> xterm.c:6372:13: error: ‘Qredisplay_dont_pause’ undeclared (first use in this function)
>>  6372 |   specbind (Qredisplay_dont_pause, Qt);
>>       |             ^~~~~~~~~~~~~~~~~~~~~
>> xterm.c:6372:13: note: each undeclared identifier is reported only once for each function it appears in
>> xfns.c: In function ‘Fx_print_frames_dialog’:
>> xfns.c:9967:13: error: ‘Qredisplay_dont_pause’ undeclared (first use in this function)
>>  9967 |   specbind (Qredisplay_dont_pause, Qt);
>>       |             ^~~~~~~~~~~~~~~~~~~~~
>> xfns.c:9967:13: note: each undeclared identifier is reported only once for each function it appears in
>> =====
>>
>> Could you have a look?
>>
>> Thanks
>
> Thanks, should be fixed now.

On a61b968ccfe8fa7e2f1fcf7667781a1c0527389a I see now with
--with-native-compilation=no --with-x-toolkit=no

=====
dispnew.c: In function ‘update_menu_bar’:
dispnew.c:3760:5: error: too many arguments to function ‘update_window’
 3760 |     update_window (XWINDOW (f->menu_bar_window), true);
      |     ^~~~~~~~~~~~~
dispnew.c:97:13: note: declared here
   97 | static void update_window (struct window *);
      |             ^~~~~~~~~~~~~
=======

And with --enable-checking=all --enable-check-lisp-object-type --with-native-compilation=yes

=======
In file included from dispnew.c:27:
In function ‘PSEUDOVECTORP’,
    inlined from ‘FRAMEP’ at lisp.h:3343:10,
    inlined from ‘FRAME_PARENT_FRAME’ at frame.h:1236:42,
    inlined from ‘root_frame’ at dispnew.c:3340:10,
    inlined from ‘is_in_matrix’ at dispnew.c:3877:24,
    inlined from ‘is_cursor_obscured’ at dispnew.c:3898:8,
    inlined from ‘terminal_cursor_magic’ at dispnew.c:3917:7,
    inlined from ‘combine_updates_for_frame’ at dispnew.c:3970:5:
lisp.h:1096:68: warning: null pointer dereference [-Wnull-dereference]
 1096 |           && ((XUNTAG (a, Lisp_Vectorlike, union vectorlike_header)->size
      |                                                                    ^
=======

Thanks

Re: master f92520d009e 1/2: Don't pause display for pending input

[Gerd Möllmann]

Andrea Corallo <acorallo@gnu.org> writes:

> On a61b968ccfe8fa7e2f1fcf7667781a1c0527389a I see now with
> --with-native-compilation=no --with-x-toolkit=no
>
> =====
> dispnew.c: In function ‘update_menu_bar’:
> dispnew.c:3760:5: error: too many arguments to function ‘update_window’
>  3760 |     update_window (XWINDOW (f->menu_bar_window), true);
>       |     ^~~~~~~~~~~~~
> dispnew.c:97:13: note: declared here
>    97 | static void update_window (struct window *);
>       |             ^~~~~~~~~~~~~
> =======

Thanks. That one should be fixed now. The joy of having n!
configurations.

>
> And with --enable-checking=all --enable-check-lisp-object-type --with-native-compilation=yes
>
> =======
> In file included from dispnew.c:27:
> In function ‘PSEUDOVECTORP’,
>     inlined from ‘FRAMEP’ at lisp.h:3343:10,
>     inlined from ‘FRAME_PARENT_FRAME’ at frame.h:1236:42,
>     inlined from ‘root_frame’ at dispnew.c:3340:10,
>     inlined from ‘is_in_matrix’ at dispnew.c:3877:24,
>     inlined from ‘is_cursor_obscured’ at dispnew.c:3898:8,
>     inlined from ‘terminal_cursor_magic’ at dispnew.c:3917:7,
>     inlined from ‘combine_updates_for_frame’ at dispnew.c:3970:5:
> lisp.h:1096:68: warning: null pointer dereference [-Wnull-dereference]
>  1096 |           && ((XUNTAG (a, Lisp_Vectorlike, union vectorlike_header)->size
>       |                                                                    ^
> =======
>
This one I don't see here, with clang 19. And I must admit I don't
understand it what it is complaining about. The function root_frame
is pretty simple:

  struct frame *
  root_frame (struct frame *f)
  {
    while (FRAME_PARENT_FRAME (f))
      f = FRAME_PARENT_FRAME (f);
    return f;
  }

and FRAME_PARENT_FFAME is

  INLINE struct frame *
  FRAME_PARENT_FRAME (struct frame *f)
  {
    return NILP (f->parent_frame) ? NULL : XFRAME (f->parent_frame);
  }

Any idea?

Re: master f92520d009e 1/2: Don't pause display for pending input

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org,  Gerd Möllmann <gerd@gnu.org>
> Date: Thu, 19 Dec 2024 13:24:41 +0100
> 
> Andrea Corallo <acorallo@gnu.org> writes:
> 
> > And with --enable-checking=all --enable-check-lisp-object-type --with-native-compilation=yes
> >
> > =======
> > In file included from dispnew.c:27:
> > In function ‘PSEUDOVECTORP’,
> >     inlined from ‘FRAMEP’ at lisp.h:3343:10,
> >     inlined from ‘FRAME_PARENT_FRAME’ at frame.h:1236:42,
> >     inlined from ‘root_frame’ at dispnew.c:3340:10,
> >     inlined from ‘is_in_matrix’ at dispnew.c:3877:24,
> >     inlined from ‘is_cursor_obscured’ at dispnew.c:3898:8,
> >     inlined from ‘terminal_cursor_magic’ at dispnew.c:3917:7,
> >     inlined from ‘combine_updates_for_frame’ at dispnew.c:3970:5:
> > lisp.h:1096:68: warning: null pointer dereference [-Wnull-dereference]
> >  1096 |           && ((XUNTAG (a, Lisp_Vectorlike, union vectorlike_header)->size
> >       |                                                                    ^
> > =======
> >
> This one I don't see here, with clang 19. And I must admit I don't
> understand it what it is complaining about. The function root_frame
> is pretty simple:
> 
>   struct frame *
>   root_frame (struct frame *f)
>   {
>     while (FRAME_PARENT_FRAME (f))
>       f = FRAME_PARENT_FRAME (f);
>     return f;
>   }
> 
> and FRAME_PARENT_FFAME is
> 
>   INLINE struct frame *
>   FRAME_PARENT_FRAME (struct frame *f)
>   {
>     return NILP (f->parent_frame) ? NULL : XFRAME (f->parent_frame);
>   }
> 
> Any idea?

I think it wants to tell you that the argument F of FRAME_PARENT_FRAME
could be NULL.

Re: master f92520d009e 1/2: Don't pause display for pending input

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org,  Gerd Möllmann <gerd@gnu.org>
>> Date: Thu, 19 Dec 2024 13:24:41 +0100
>> 
>> Andrea Corallo <acorallo@gnu.org> writes:
>> 
>> > And with --enable-checking=all --enable-check-lisp-object-type --with-native-compilation=yes
>> >
>> > =======
>> > In file included from dispnew.c:27:
>> > In function ‘PSEUDOVECTORP’,
>> >     inlined from ‘FRAMEP’ at lisp.h:3343:10,
>> >     inlined from ‘FRAME_PARENT_FRAME’ at frame.h:1236:42,
>> >     inlined from ‘root_frame’ at dispnew.c:3340:10,
>> >     inlined from ‘is_in_matrix’ at dispnew.c:3877:24,
>> >     inlined from ‘is_cursor_obscured’ at dispnew.c:3898:8,
>> >     inlined from ‘terminal_cursor_magic’ at dispnew.c:3917:7,
>> >     inlined from ‘combine_updates_for_frame’ at dispnew.c:3970:5:
>> > lisp.h:1096:68: warning: null pointer dereference [-Wnull-dereference]
>> >  1096 |           && ((XUNTAG (a, Lisp_Vectorlike, union vectorlike_header)->size
>> >       |                                                                    ^
>> > =======
>> >
>> This one I don't see here, with clang 19. And I must admit I don't
>> understand it what it is complaining about. The function root_frame
>> is pretty simple:
>> 
>>   struct frame *
>>   root_frame (struct frame *f)
>>   {
>>     while (FRAME_PARENT_FRAME (f))
>>       f = FRAME_PARENT_FRAME (f);
>>     return f;
>>   }
>> 
>> and FRAME_PARENT_FFAME is
>> 
>>   INLINE struct frame *
>>   FRAME_PARENT_FRAME (struct frame *f)
>>   {
>>     return NILP (f->parent_frame) ? NULL : XFRAME (f->parent_frame);
>>   }
>> 
>> Any idea?
>
> I think it wants to tell you that the argument F of FRAME_PARENT_FRAME
> could be NULL.

The only explanation I can find is that GCC seems this

  #define SELECTED_FRAME()				\
       ((FRAMEP (selected_frame)				\
         && FRAME_LIVE_P (XFRAME (selected_frame)))	\
        ? XFRAME (selected_frame)				\
        : (emacs_abort (), (struct frame *) 0))

and doesn't realize that it actually never returns NULL. That's from up
the stack in is_cursor_obscured.

Beats me, sorry.

Re: master f92520d009e 1/2: Don't pause display for pending input

[Andrea Corallo]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Andrea Corallo <acorallo@gnu.org> writes:
>
>> On a61b968ccfe8fa7e2f1fcf7667781a1c0527389a I see now with
>> --with-native-compilation=no --with-x-toolkit=no
>>
>> =====
>> dispnew.c: In function ‘update_menu_bar’:
>> dispnew.c:3760:5: error: too many arguments to function ‘update_window’
>>  3760 |     update_window (XWINDOW (f->menu_bar_window), true);
>>       |     ^~~~~~~~~~~~~
>> dispnew.c:97:13: note: declared here
>>    97 | static void update_window (struct window *);
>>       |             ^~~~~~~~~~~~~
>> =======
>
> Thanks. That one should be fixed now. The joy of having n!
> configurations.

Thanks.

PS if this was a release branch I could have tracked it with my
testbench before the merge (sorry for being tedious really :)

  Andrea

Re: master f92520d009e 1/2: Don't pause display for pending input

[Gerd Möllmann]

Andrea Corallo <acorallo@gnu.org> writes:

> PS if this was a release branch I could have tracked it with my
> testbench before the merge (sorry for being tedious really :)

You're aiming at scratch/igc? :-)

Re: master f92520d009e 1/2: Don't pause display for pending input

[Andrea Corallo]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Andrea Corallo <acorallo@gnu.org> writes:
>
>> PS if this was a release branch I could have tracked it with my
>> testbench before the merge (sorry for being tedious really :)
>
> You're aiming at scratch/igc? :-)

I'm a dreamer 😇

Re: master f92520d009e 1/2: Don't pause display for pending input

[Gerd Möllmann]

Andrea Corallo <acorallo@gnu.org> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Andrea Corallo <acorallo@gnu.org> writes:
>>
>>> PS if this was a release branch I could have tracked it with my
>>> testbench before the merge (sorry for being tedious really :)
>>
>> You're aiming at scratch/igc? :-)
>
> I'm a dreamer 😇

Hehe :-)