bug#55412: 28.1; In Emacs 28.1, using ':eval' in 'frame-title-format' doesn't work properly

[martin rudalics <rudalics@gmx.at>]

 > with_window_selected is not re-entrant, since it uses a static data
 > structure.  Is this deliberate, or just a case of not yet making it
 > re-entrant?

You mean Vwith_window_selected_vector.  It's deliberate, just
transferred here from 'format_mode_line_unwind_data' which did the same.
Apparently the overhead for allocating a new vector during redisplay was
deemed as too expensive for making this re-entrant.

 > I get nervous when I see "selected_frame = ...;" or "selected_window =
 > ....;" outside of do_switch_frame or the corresponding window function,
 > since it means selected_{frame,window} have been set without regard to
 > all the other things which must be kept in synch with them.  At the
 > moment, all these "wild" settings of selected_frame are in xdisp.c,
 > which I suppose has special reasons for them.

What concerns me is that xdisp.c sets selected_window and selected_frame
in the first place (also due to my attempts to fix Bug#39977).  My patch
removes them all.  This part of the redisplay code has been mended over
and over again in the past years, adding all those "special reasons" you
mention.

 > with_window_selected would add another "wild" setting of selected_frame,
 > this one outside of xdisp.c (in window.c) and I ask whether or not this
 > is a good thing.

The canonical function to set selected_window is the static
select_window_1 and that's in window.c (with_window_selected sets
selected_window too but that's only a minor optimization).

 > I think the root of the problem your patch is trying to solve is that
 > the same C code is used for implementing C-x 5 o and lower level,
 > temporary, frame switches.  If this is the case, a good way of
 > proceeding would be to refactor do_switch_frame into a function
 > appropriate for lower level C calls, and the remainder for C-x 5 o.  For
 > example, the call to move_minibuffers_onto_frame is clearly needed for
 > C-x 5 o, but is a complicated nuisance for lower level C.  With such a
 > new extracted C function, we could replace the "selected_frame = ...;"
 > in xdisp.c by calls to this function.  Maybe.

The redisplay code has to temporarily select a window also when there's
only one frame around in which case do_switch_frame wouldn't enter the
picture at all.  with_window_selected handles both cases.

 > I don't think redisplay should be calling Fselect_window or
 > do_switch_frame at all.  Instead we should call (not yet existing) lower
 > level functions.

That's what with_window_selected and its unwind form have been designed
for.  Basically, any such code has to guarantee invariants like:

- The selected frame's selected window is the selected window.

- Code run within with_window_selected can rely on the fact that the
   selected window's buffer is current.

Moreover, the code has to guarantee that selecting a window correctly
sets up point from the window's point and stores the old point in the
previously selected window#s point.  And it obviously has to guarantee
that the involved buffers, windows and frames are alive when switching
to and back from them - things redisplay had always problems with
(confer Bug#47244).  Not reliably doing all these things in one place
easily produces bugs that show up only in sessions that run for a long
time and are consequently very difficult to debug.

One major advantage of with_window_selected is that then one can debug
functions like 'select-window', 'select-frame' or 'redirect-frame-focus'
without being continuously hassled by redisplay which here accounts for
nine out of ten calls of these functions at least and eventually caused
me to abandon a number of fixes for say Bug#32777, Bug#39977, Bug#24500
and Bug#24803.

martin