bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Aaron Jensen]

(setq header-line-format '(:eval (format-mode-line "")))

This causes Emacs to spin as of commit:
4f66cbbfe520ee31ef26676e09a926217d9736fe

After some time, it will segfault.


In GNU Emacs 30.0.50 (build 1, aarch64-apple-darwin22.4.0, NS
 appkit-2299.50 Version 13.3.1 (a) (Build 22E772610a)) of 2023-05-19
 built on Aarons-Laptop.local
Windowing system distributor 'Apple', version 10.3.2299
System Description:  macOS 13.4

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Fri, 09 Jun 2023 21:09:45 -0400
> 
> 
> (setq header-line-format '(:eval (format-mode-line "")))
> 
> This causes Emacs to spin as of commit:
> 4f66cbbfe520ee31ef26676e09a926217d9736fe
> 
> After some time, it will segfault.

It causes infinite recursion, since format-mode-line also calls
window_wants_header_line (indirectly).

But what is the purpose of such a strange (to use a civilized word)
setting of header-line-format?  Why do you need :eval at all in this
case?

IOW, why not say "don't do that" and be done?

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Eli Zaretskii]

> Cc: 63988@debbugs.gnu.org
> Date: Sat, 10 Jun 2023 09:47:29 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Fri, 09 Jun 2023 21:09:45 -0400
> > 
> > 
> > (setq header-line-format '(:eval (format-mode-line "")))
> > 
> > This causes Emacs to spin as of commit:
> > 4f66cbbfe520ee31ef26676e09a926217d9736fe
> > 
> > After some time, it will segfault.
> 
> It causes infinite recursion, since format-mode-line also calls
> window_wants_header_line (indirectly).
> 
> But what is the purpose of such a strange (to use a civilized word)
> setting of header-line-format?  Why do you need :eval at all in this
> case?
> 
> IOW, why not say "don't do that" and be done?

For now, I installed a semi-kludgey fix.

However, I wonder whether we should rethink this minor feature.
Perhaps this minor convenience is not worth the complications?
window_wants_header_line is called in many places, all of which can
now evaluate arbitrary Lisp, and all of which can now GC.  I've
audited the various callers, and didn't see anything obvious that
could cause problems with calling Lisp or GC in those places, but I
could have missed something.

This is actually a general issue with Emacs: we keep piling one minor
feature upon another, and don't always reflect on the hard-to-maintain
monster that creates.  We have already a couple of areas in the code
base where we are afraid of making changes, because we don't have a
good understanding of the complicated state variables there.  Maybe we
should reject such minor features instead of keeping doing what we
have been doing?

So maybe we should declare this feature a failed experiment and remove
it?

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 63988@debbugs.gnu.org
>> Date: Sat, 10 Jun 2023 09:47:29 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> 
>> > From: Aaron Jensen <aaronjensen@gmail.com>
>> > Date: Fri, 09 Jun 2023 21:09:45 -0400
>> > 
>> > 
>> > (setq header-line-format '(:eval (format-mode-line "")))
>> > 
>> > This causes Emacs to spin as of commit:
>> > 4f66cbbfe520ee31ef26676e09a926217d9736fe
>> > 
>> > After some time, it will segfault.
>> 
>> It causes infinite recursion, since format-mode-line also calls
>> window_wants_header_line (indirectly).
>> 
>> But what is the purpose of such a strange (to use a civilized word)
>> setting of header-line-format?  Why do you need :eval at all in this
>> case?
>> 
>> IOW, why not say "don't do that" and be done?
>
> For now, I installed a semi-kludgey fix.
>
> However, I wonder whether we should rethink this minor feature.
> Perhaps this minor convenience is not worth the complications?
> window_wants_header_line is called in many places, all of which can
> now evaluate arbitrary Lisp, and all of which can now GC.  I've
> audited the various callers, and didn't see anything obvious that
> could cause problems with calling Lisp or GC in those places, but I
> could have missed something.
>
> This is actually a general issue with Emacs: we keep piling one minor
> feature upon another, and don't always reflect on the hard-to-maintain
> monster that creates.  We have already a couple of areas in the code
> base where we are afraid of making changes, because we don't have a
> good understanding of the complicated state variables there.  Maybe we
> should reject such minor features instead of keeping doing what we
> have been doing?
>
> So maybe we should declare this feature a failed experiment and remove
> it?

FWIW I think that make sense, but IMO it'd be best to only remove the
treatment of `:eval` in `window_wants_header_line`, and keep the new
treatment of `header-line-format` being a cons cell with a void or
nil-valued variable car.  That's still useful because it works well with
minor mode variables, and it's less risky as it doesn't involve
evaluating arbitrary lisp, just inspecting a variable.

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: aaronjensen@gmail.com,  63988@debbugs.gnu.org,  Stefan Monnier
>  <monnier@iro.umontreal.ca>
> Date: Sat, 10 Jun 2023 12:07:22 +0300
> 
> > So maybe we should declare this feature a failed experiment and remove
> > it?
> 
> FWIW I think that make sense, but IMO it'd be best to only remove the
> treatment of `:eval` in `window_wants_header_line`, and keep the new
> treatment of `header-line-format` being a cons cell with a void or
> nil-valued variable car.  That's still useful because it works well with
> minor mode variables, and it's less risky as it doesn't involve
> evaluating arbitrary lisp, just inspecting a variable.

Why would it make sense to leave the non-nil car case?

It sounds like the consensus here is that indeed this feature is not
worth the complications, and so, unless I hear some good reasons not
to do so, I intend to delete it in a week's time.

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Aaron Jensen]

On Thu, Jun 15, 2023 at 1:58 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Eshel Yaron <me@eshelyaron.com>
> > Cc: aaronjensen@gmail.com,  63988@debbugs.gnu.org,  Stefan Monnier
> >  <monnier@iro.umontreal.ca>
> > Date: Sat, 10 Jun 2023 12:07:22 +0300
> >
> > > So maybe we should declare this feature a failed experiment and remove
> > > it?
> >
> > FWIW I think that make sense, but IMO it'd be best to only remove the
> > treatment of `:eval` in `window_wants_header_line`, and keep the new
> > treatment of `header-line-format` being a cons cell with a void or
> > nil-valued variable car.  That's still useful because it works well with
> > minor mode variables, and it's less risky as it doesn't involve
> > evaluating arbitrary lisp, just inspecting a variable.
>
> Why would it make sense to leave the non-nil car case?
>
> It sounds like the consensus here is that indeed this feature is not
> worth the complications, and so, unless I hear some good reasons not
> to do so, I intend to delete it in a week's time.

I strongly support this. It's a major performance regression for me
and it seems to correspond with crashes on window resizing:

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib               0x191054724 __pthread_kill + 8
1   libsystem_pthread.dylib               0x19108bc28 pthread_kill + 288
2   libsystem_c.dylib                     0x190f6246c raise + 32
3   Emacs                                 0x100a215b0
terminate_due_to_signal + 212
4   Emacs                                 0x100a21dfc emacs_abort + 20
5   Emacs                                 0x1009f0594 ns_term_shutdown + 132
6   Emacs                                 0x1008dcc90 shut_down_emacs + 332
7   Emacs                                 0x100a21578
terminate_due_to_signal + 156
8   Emacs                                 0x1008fd4bc
deliver_fatal_thread_signal + 128
9   Emacs                                 0x1008fee80 handle_sigsegv + 64
10  libsystem_platform.dylib             0x1910baa24 _sigtramp + 56
11  Emacs                                 0x10083a90c start_display + 56
12  Emacs                                 0x10085010c try_window + 144
13  Emacs                                 0x10086c88c display_echo_area_1 + 136
14  Emacs                                 0x1008496dc
with_echo_area_buffer + 784
15  Emacs                                 0x1008485bc echo_area_display + 268
16  Emacs                                 0x1008483bc message3_nolog + 548
17  Emacs                                 0x100847f5c message3 + 132
18  Emacs                                 0x100952470 Fmessage + 68
19  Emacs                                 0x100958074 eval_sub + 1980
20  Emacs                                 0x10095824c Fprogn + 48
21  Emacs                                 0x10095d728 funcall_lambda + 1276
22  Emacs                                 0x10095c284 apply_lambda + 280
23  Emacs                                 0x100957da8 eval_sub + 1264
24  Emacs                                 0x1009581c8 Fand + 60
25  Emacs                                 0x100957f7c eval_sub + 1732
26  Emacs                                 0x1009593f0 FletX + 268
27  Emacs                                 0x100957f7c eval_sub + 1732
28  Emacs                                 0x100957ed0 eval_sub + 1560
29  Emacs                                 0x100957ed0 eval_sub + 1560
30  Emacs                                 0x100957ed0 eval_sub + 1560
31  Emacs                                 0x10095824c Fprogn + 48
32  Emacs                                 0x10095d728 funcall_lambda + 1276
33  Emacs                                 0x10095c284 apply_lambda + 280
34  Emacs                                 0x100957da8 eval_sub + 1264
35  Emacs                                 0x10095974c Flet + 316
36  Emacs                                 0x100957f7c eval_sub + 1732
37  Emacs                                 0x100958034 eval_sub + 1916
38  Emacs                                 0x10095824c Fprogn + 48
39  Emacs                                 0x10095d728 funcall_lambda + 1276
40  Emacs                                 0x100959dc8 Ffuncall + 316
41  Emacs                                 0x100958074 eval_sub + 1980
42  Emacs                                 0x10095824c Fprogn + 48
43  Emacs                                 0x100957f7c eval_sub + 1732
44  Emacs                                 0x10095824c Fprogn + 48
45  Emacs                                 0x10095d728 funcall_lambda + 1276
46  Emacs                                 0x100959dc8 Ffuncall + 316
47  Emacs                                 0x1009677d4 mapcar1 + 328
48  Emacs                                 0x1009675b4 Fmapconcat + 460
49  Emacs                                 0x1009580d8 eval_sub + 2080
50  Emacs                                 0x1009593f0 FletX + 268
51  Emacs                                 0x100957f7c eval_sub + 1732
52  Emacs                                 0x10095c070 Feval + 84
53  Emacs                                 0x100959dc8 Ffuncall + 316
54  Emacs                                 0x10095ab74
internal_condition_case_n + 156
55  Emacs                                 0x100842d84 safe__call + 296
56  Emacs                                 0x100842e40 safe__call1 + 36
57  Emacs                                 0x100842e68
safe_eval_inhibit_quit + 28
58  Emacs                                 0x10088d47c
null_header_line_format + 388
59  Emacs                                 0x100881800
window_wants_header_line + 296
60  Emacs                                 0x100839e0c window_box_height + 860
61  Emacs                                 0x100829838
required_matrix_height + 72
62  Emacs                                 0x100829908
allocate_matrices_for_window_redisplay + 148
63  Emacs                                 0x100823728 adjust_frame_glyphs + 88
64  Emacs                                 0x10088f638
Fset_window_configuration + 4068
65  tab-bar-f81d329c-3b6a0fc2.eln        0x107163bc8
F7461622d6261722d73656c6563742d746162_tab_bar_select_tab_0 + 868
66  Emacs                                 0x100959dc8 Ffuncall + 316
67  tab-bar-f81d329c-3b6a0fc2.eln        0x1071641d4
F7461622d6261722d7377697463682d746f2d6e6578742d746162_tab_bar_switch_to_next_tab_0
+ 260
68  Emacs                                 0x100959dc8 Ffuncall + 316
69  Emacs                                 0x10095592c
Ffuncall_interactively + 68
70  Emacs                                 0x100959dc8 Ffuncall + 316
71  Emacs                                 0x100956a08 Fcall_interactively + 4292
72  simple-fab5b0cf-e1c8f2a9.eln         0x102b54b0c
F636f6d6d616e642d65786563757465_command_execute_0 + 652

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Eli Zaretskii]

> Cc: 63988@debbugs.gnu.org, aaronjensen@gmail.com, monnier@iro.umontreal.ca
> Date: Thu, 15 Jun 2023 08:59:09 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> It sounds like the consensus here is that indeed this feature is not
> worth the complications, and so, unless I hear some good reasons not
> to do so, I intend to delete it in a week's time.

Time's up, so I've now reverted all the commits that dealt with this
feature, and I'm marking this bug done.

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Eshel Yaron <me@eshelyaron.com>
>> Cc: aaronjensen@gmail.com,  63988@debbugs.gnu.org,  Stefan Monnier
>>  <monnier@iro.umontreal.ca>
>> Date: Sat, 10 Jun 2023 12:07:22 +0300
>> 
>> > So maybe we should declare this feature a failed experiment and remove
>> > it?
>> 
>> FWIW I think that make sense, but IMO it'd be best to only remove the
>> treatment of `:eval` in `window_wants_header_line`, and keep the new
>> treatment of `header-line-format` being a cons cell with a void or
>> nil-valued variable car.  That's still useful because it works well with
>> minor mode variables, and it's less risky as it doesn't involve
>> evaluating arbitrary lisp, just inspecting a variable.
>
> Why would it make sense to leave the non-nil car case?
>
> It sounds like the consensus here is that indeed this feature is not
> worth the complications, and so, unless I hear some good reasons not
> to do so, I intend to delete it in a week's time.

Ugh, I am only just now seeing this... I would have appreciated a CC
since I'm the one who requested this feature, or maybe a mail sent to
the original bug report...

Anyway, the reason for this feature remains what is described in
bug#63825: displaying a header line only when there is information to be
displayed in the header line.

Concretely, for which-function-mode: it is nice to have
which-func-format displayed in the header line, but in some kinds of
buffers it does not have any information to display.  When a window is
displaying such a buffer, I'd prefer to not have a header line, because
the header line will just be empty.

This use case has no need for the full-fledged :eval mode of this
feature.  It works fine with the non-nil car case.

How else should I achieve this?

I could add this functionality to which-func-mode directly: it could
learn to set header-line-format buffer-locally only in buffers that
support which-func-mode.  Would that be acceptable?  If so I'll send a
patch for that.

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: Eshel Yaron <me@eshelyaron.com>,  63988@debbugs.gnu.org,
>    aaronjensen@gmail.com,  monnier@iro.umontreal.ca
> Date: Wed, 05 Jul 2023 11:30:41 -0400
> 
> > It sounds like the consensus here is that indeed this feature is not
> > worth the complications, and so, unless I hear some good reasons not
> > to do so, I intend to delete it in a week's time.
> 
> Ugh, I am only just now seeing this... I would have appreciated a CC
> since I'm the one who requested this feature, or maybe a mail sent to
> the original bug report...
> 
> Anyway, the reason for this feature remains what is described in
> bug#63825: displaying a header line only when there is information to be
> displayed in the header line.

The discussion indicated that the reason for the feature, while it is
valid, is not serious enough to justify the complications described in
the thread.

> Concretely, for which-function-mode: it is nice to have
> which-func-format displayed in the header line, but in some kinds of
> buffers it does not have any information to display.  When a window is
> displaying such a buffer, I'd prefer to not have a header line, because
> the header line will just be empty.
> 
> This use case has no need for the full-fledged :eval mode of this
> feature.  It works fine with the non-nil car case.

That the car is nil doesn't necessarily mean that the header will not
show something.

> I could add this functionality to which-func-mode directly: it could
> learn to set header-line-format buffer-locally only in buffers that
> support which-func-mode.  Would that be acceptable?  If so I'll send a
> patch for that.

If you mean that some Lisp will set header-line-format nil when some
condition is true, I think it should be fine, assuming you can arrange
for this Lisp to run whenever the condition could change.

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> It causes infinite recursion, since format-mode-line also calls
>> window_wants_header_line (indirectly).

I wonder why `format-mode-line` calls `window_wants_header_line`.
Is there a deep technical reason, or just an accident of how we
currently implement the feature?

> However, I wonder whether we should rethink this minor feature.
> Perhaps this minor convenience is not worth the complications?

IIUC This is talking about commit
4f66cbbfe520ee31ef26676e09a926217d9736fe which extends the special
treatment of a nil `header-line-format` to also hide the header line
when its value is "equivalent" to nil.

In the past I've wished for a non-nil mode-line that is not displayed
because it's equivalent to nil, so I think the feature makes sense, but
I agree it's not super important.  Maybe if we want to make it work
well, the better solution is to memoize the computation of
(format-mode-line header-line-format) so that it's called at most once
per redisplay?


        Stefan

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: aaronjensen@gmail.com,  Eshel Yaron <me@eshelyaron.com>,
>   63988@debbugs.gnu.org
> Date: Sat, 10 Jun 2023 12:16:45 -0400
> 
> >> It causes infinite recursion, since format-mode-line also calls
> >> window_wants_header_line (indirectly).
> 
> I wonder why `format-mode-line` calls `window_wants_header_line`.
> Is there a deep technical reason, or just an accident of how we
> currently implement the feature?

format-mode-line calls init_iterator (because the formatting is done
by display code), and init_iterator calls window_wants_header_line
because it needs to know the general layout of the window up front.

We could introduce some knobs to perhaps avoid this in the particular
case of format-mode-line, but the :eval form in header-line-format can
call any Lisp, not necessarily format-mode-line.  And the Lisp code
called from :eval could easily call some primitive that uses the
display code, for example vertical-motion or window-text-pixel-size or
something else.  Going over all those places and making sure
init_iterator will not call window_wants_header_line in those cases is
not my idea of fun, even if it is practical to discern between the
cases where it does and doesn't need that.

> > However, I wonder whether we should rethink this minor feature.
> > Perhaps this minor convenience is not worth the complications?
> 
> IIUC This is talking about commit
> 4f66cbbfe520ee31ef26676e09a926217d9736fe which extends the special
> treatment of a nil `header-line-format` to also hide the header line
> when its value is "equivalent" to nil.
> 
> In the past I've wished for a non-nil mode-line that is not displayed
> because it's equivalent to nil, so I think the feature makes sense, but
> I agree it's not super important.  Maybe if we want to make it work
> well, the better solution is to memoize the computation of
> (format-mode-line header-line-format) so that it's called at most once
> per redisplay?

That is possible, and I thought about it as well, but it isn't easy.
Once again: the display code is many times called for purposes other
than display, and in at least some of those cases it does need to know
whether there is a header-line, because the layout and the metrics of
the text on display depend on that.  So the question of when to record
the fact of header-line existence for a particular window -- that
question doesn't have an easy answer.  It is easy to compute that at
the beginning of redisplay_window, but what about the following
scenario in some Lisp program:

  . do some calculation that affects header-line existence
  . call window-text-pixel-size

and its many variations?  Are we going to sprinkle the code with calls
to a function that calculates the header-line and records the result
somewhere (perhaps in a window) before each call to init_iterator and
start_display, and if so, how to avoid potential recursion like the
one in this case?

Or maybe we want to use the variable-watcher feature to do that
whenever header-line-format is modified?  That is also far from being
fun, and could cause performance degradation, as some people, I hear,
use header-line to display the mode-line information.

Bottom line: this sounds like a minor convenience feature that causes
major headaches to implement, because once you allow evaluation of
arbitrary Lisp, all bets are basically off.

Let's admit it: the current Emacs display code was not designed to
support this, so it's a little wonder we face an uphill battle.  And
maybe we should just admit defeat and say we cannot support it until
and unless the display engine is redesigned with this goal in mind.

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Aaron Jensen]

On Sat, Jun 10, 2023 at 1:42 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Cc: aaronjensen@gmail.com,  Eshel Yaron <me@eshelyaron.com>,
> >   63988@debbugs.gnu.org
> > Date: Sat, 10 Jun 2023 12:16:45 -0400
> >
> > >> It causes infinite recursion, since format-mode-line also calls
> > >> window_wants_header_line (indirectly).
> >
> > I wonder why `format-mode-line` calls `window_wants_header_line`.
> > Is there a deep technical reason, or just an accident of how we
> > currently implement the feature?
>
> format-mode-line calls init_iterator (because the formatting is done
> by display code), and init_iterator calls window_wants_header_line
> because it needs to know the general layout of the window up front.
>
> We could introduce some knobs to perhaps avoid this in the particular
> case of format-mode-line, but the :eval form in header-line-format can
> call any Lisp, not necessarily format-mode-line.  And the Lisp code
> called from :eval could easily call some primitive that uses the
> display code, for example vertical-motion or window-text-pixel-size or
> something else.  Going over all those places and making sure
> init_iterator will not call window_wants_header_line in those cases is
> not my idea of fun, even if it is practical to discern between the
> cases where it does and doesn't need that.
>
> > > However, I wonder whether we should rethink this minor feature.
> > > Perhaps this minor convenience is not worth the complications?
> >
> > IIUC This is talking about commit
> > 4f66cbbfe520ee31ef26676e09a926217d9736fe which extends the special
> > treatment of a nil `header-line-format` to also hide the header line
> > when its value is "equivalent" to nil.
> >
> > In the past I've wished for a non-nil mode-line that is not displayed
> > because it's equivalent to nil, so I think the feature makes sense, but
> > I agree it's not super important.  Maybe if we want to make it work
> > well, the better solution is to memoize the computation of
> > (format-mode-line header-line-format) so that it's called at most once
> > per redisplay?
>
> That is possible, and I thought about it as well, but it isn't easy.
> Once again: the display code is many times called for purposes other
> than display, and in at least some of those cases it does need to know
> whether there is a header-line, because the layout and the metrics of
> the text on display depend on that.  So the question of when to record
> the fact of header-line existence for a particular window -- that
> question doesn't have an easy answer.  It is easy to compute that at
> the beginning of redisplay_window, but what about the following
> scenario in some Lisp program:
>
>   . do some calculation that affects header-line existence
>   . call window-text-pixel-size
>
> and its many variations?  Are we going to sprinkle the code with calls
> to a function that calculates the header-line and records the result
> somewhere (perhaps in a window) before each call to init_iterator and
> start_display, and if so, how to avoid potential recursion like the
> one in this case?
>
> Or maybe we want to use the variable-watcher feature to do that
> whenever header-line-format is modified?  That is also far from being
> fun, and could cause performance degradation, as some people, I hear,
> use header-line to display the mode-line information.
>
> Bottom line: this sounds like a minor convenience feature that causes
> major headaches to implement, because once you allow evaluation of
> arbitrary Lisp, all bets are basically off.
>
> Let's admit it: the current Emacs display code was not designed to
> support this, so it's a little wonder we face an uphill battle.  And
> maybe we should just admit defeat and say we cannot support it until
> and unless the display engine is redesigned with this goal in mind.

I'm personally fine with this. I also wonder if the initial use-case
that led to this feature could be handled instead by controlling the
header line format when whatever factor changes that would determine
whether or not to display a header line. I know I have to do this with
mode line now that my mode line is typically nil, but I use it
sometimes to display things like number of matches.

Aaron

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> I wonder why `format-mode-line` calls `window_wants_header_line`.
>> Is there a deep technical reason, or just an accident of how we
>> currently implement the feature?
> format-mode-line calls init_iterator (because the formatting is done
> by display code), and init_iterator calls window_wants_header_line
> because it needs to know the general layout of the window up front.

Ah, I see.

> We could introduce some knobs to perhaps avoid this in the particular
> case of format-mode-line, but the :eval form in header-line-format can
> call any Lisp, not necessarily format-mode-line.

There's no doubt that in the most general case the arbitrary ELisp code
run from `header-line-format` can cause enough redisplay work to require
computation of `header-line-format`, indeed.

>> In the past I've wished for a non-nil mode-line that is not displayed
>> because it's equivalent to nil, so I think the feature makes sense, but
>> I agree it's not super important.  Maybe if we want to make it work
>> well, the better solution is to memoize the computation of
>> (format-mode-line header-line-format) so that it's called at most once
>> per redisplay?
>
> That is possible, and I thought about it as well, but it isn't easy.

Agreed.  I can see two benefits:

- try and handle the recursion by marking the header-line as "wanted"
  while we compute it so recursive calls to `window_wants_header_line`
  just return `true` without actually doing the work recursively.
- try and reduce the cost of re-computing it every time.

But obviously, like all memoization/caching schemes, the hard part is to
figure out how/when to flush the obsolete info.  It's definitely not easy.

> Bottom line: this sounds like a minor convenience feature that causes
> major headaches to implement, because once you allow evaluation of
> arbitrary Lisp, all bets are basically off.

I can't remember the details of when I wanted a non-nil
`mode-line-format` to be equivalent to nil, but IIRC I worked around the
problem with ad-hoc hooks at various places to set/reset
`mode-line-format` to nil and back according to whether a mode line
should be displayed.

Still, I can see cases where we may want a header-line that's only
displayed in some specific *windows*, so the buffer's value can't be nil
(for that window) but shouldn't be non-nil (for the other windows
displaying that buffer).  In the absence of the problematic commit we
can't handle such cases.

But we've lived with such a limitation for almost 40 years, so I'm fine
with removing that feature rather than trying to make it work.
We'll want to ad some comments in the code to try and avoid someone else
go through the same process, tho.


        Stefan

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Aaron Jensen]

On Sat, Jun 10, 2023 at 2:47 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Fri, 09 Jun 2023 21:09:45 -0400
> >
> >
> > (setq header-line-format '(:eval (format-mode-line "")))
> >
> > This causes Emacs to spin as of commit:
> > 4f66cbbfe520ee31ef26676e09a926217d9736fe
> >
> > After some time, it will segfault.
>
> It causes infinite recursion, since format-mode-line also calls
> window_wants_header_line (indirectly).
>
> But what is the purpose of such a strange (to use a civilized word)
> setting of header-line-format?  Why do you need :eval at all in this
> case?
>
> IOW, why not say "don't do that" and be done?

It's a minimal repro for an issue I encountered with a package that does this:

https://github.com/rougier/nano-modeline/blob/master/nano-modeline.el#L532

It's how this modeline/header line adds line number/cursor position to
a more complicated line. As I understand it, it has to format it first
in order to use its width to properly right align it:

https://github.com/rougier/nano-modeline/blob/master/nano-modeline.el#L278

Is there a better way to do this?

> So maybe we should declare this feature a failed experiment and remove
> it?

I'll admit I don't really understand the change. Is it actually
evaluating the cdr of the eval form up to two additional times to in
order to determine whether or not to display the headerline at all?
Wouldn't this have performance implications?

Aaron

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Sat, 10 Jun 2023 07:08:20 -0400
> Cc: 63988@debbugs.gnu.org
> 
> > But what is the purpose of such a strange (to use a civilized word)
> > setting of header-line-format?  Why do you need :eval at all in this
> > case?
> >
> > IOW, why not say "don't do that" and be done?
> 
> It's a minimal repro for an issue I encountered with a package that does this:
> 
> https://github.com/rougier/nano-modeline/blob/master/nano-modeline.el#L532
> 
> It's how this modeline/header line adds line number/cursor position to
> a more complicated line. As I understand it, it has to format it first
> in order to use its width to properly right align it:
> 
> https://github.com/rougier/nano-modeline/blob/master/nano-modeline.el#L278
> 
> Is there a better way to do this?

Use just 'format'?  I still don't understand why they use
format-mode-line there.

> > So maybe we should declare this feature a failed experiment and remove
> > it?
> 
> I'll admit I don't really understand the change. Is it actually
> evaluating the cdr of the eval form up to two additional times to in
> order to determine whether or not to display the headerline at all?

Only two?

> Wouldn't this have performance implications?

Probably, although I wouldn't expect the performance to suffer too
much.  Header-line is rarely used.

But yes, that's one more implication to consider when introducing such
minor convenience features.

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Aaron Jensen]

On Sat, Jun 10, 2023 at 7:22 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Sat, 10 Jun 2023 07:08:20 -0400
> > Cc: 63988@debbugs.gnu.org
> >
> > > But what is the purpose of such a strange (to use a civilized word)
> > > setting of header-line-format?  Why do you need :eval at all in this
> > > case?
> > >
> > > IOW, why not say "don't do that" and be done?
> >
> > It's a minimal repro for an issue I encountered with a package that does this:
> >
> > https://github.com/rougier/nano-modeline/blob/master/nano-modeline.el#L532
> >
> > It's how this modeline/header line adds line number/cursor position to
> > a more complicated line. As I understand it, it has to format it first
> > in order to use its width to properly right align it:
> >
> > https://github.com/rougier/nano-modeline/blob/master/nano-modeline.el#L278
> >
> > Is there a better way to do this?
>
> Use just 'format'?  I still don't understand why they use
> format-mode-line there.

Unless I'm mistaken, the closest equivalent to (format-modeline
"%l:%c) is something like:

(format "%d:%d" (line-number-at-pos) (current-column))

It seems like that behaves the same in narrowed buffers and not, so I
can probably use that instead.

> > > So maybe we should declare this feature a failed experiment and remove
> > > it?
> >
> > I'll admit I don't really understand the change. Is it actually
> > evaluating the cdr of the eval form up to two additional times to in
> > order to determine whether or not to display the headerline at all?
>
> Only two?

Well, 2 per render. I'm just guessing at reading the diff.

> > Wouldn't this have performance implications?
>
> Probably, although I wouldn't expect the performance to suffer too
> much.  Header-line is rarely used.

By some it's rarely used. By me and others it's used often as I use it
in place of modeline. I prefer the buffer name to be at the top.

> But yes, that's one more implication to consider when introducing such
> minor convenience features.

I'd personally prefer performance to being able to hide the header
line optionally, though I still don't really understand the change and
why the one time it's eval'd in order to get the actual text/format
can't be used to determine whether or not to show the header line.

Aaron

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Sat, 10 Jun 2023 07:59:30 -0400
> Cc: 63988@debbugs.gnu.org
> 
> > > > So maybe we should declare this feature a failed experiment and remove
> > > > it?
> > >
> > > I'll admit I don't really understand the change. Is it actually
> > > evaluating the cdr of the eval form up to two additional times to in
> > > order to determine whether or not to display the headerline at all?
> >
> > Only two?
> 
> Well, 2 per render. I'm just guessing at reading the diff.

window_wants_header_line is called in many other places as well.

> I'd personally prefer performance to being able to hide the header
> line optionally

What's wrong with setting header-line-format to nil?

> though I still don't really understand the change and
> why the one time it's eval'd in order to get the actual text/format
> can't be used to determine whether or not to show the header line.

Mainly because the window layout decisions need to know whether there
will be a header line before it's actually displayed.

bug#63988: 30.0.50; Recent header line format changes cause spin/seg fault with format-mode-line

[Aaron Jensen]

On Sat, Jun 10, 2023 at 9:04 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Sat, 10 Jun 2023 07:59:30 -0400
> > Cc: 63988@debbugs.gnu.org
> >
> > > > > So maybe we should declare this feature a failed experiment and remove
> > > > > it?
> > > >
> > > > I'll admit I don't really understand the change. Is it actually
> > > > evaluating the cdr of the eval form up to two additional times to in
> > > > order to determine whether or not to display the headerline at all?
> > >
> > > Only two?
> >
> > Well, 2 per render. I'm just guessing at reading the diff.
>
> window_wants_header_line is called in many other places as well.
>
> > I'd personally prefer performance to being able to hide the header
> > line optionally
>
> What's wrong with setting header-line-format to nil?

What I meant is that I have no use for this feature and that I'd
rather be required to set it to nil to hide it. That's me personally
though, I don't know the impetus for the original addition.

> > though I still don't really understand the change and
> > why the one time it's eval'd in order to get the actual text/format
> > can't be used to determine whether or not to show the header line.
>
> Mainly because the window layout decisions need to know whether there
> will be a header line before it's actually displayed.

Ah, that makes sense.