bug#74091: 31.0.50; string-pixel-width in mode line disables region

[Eli Zaretskii <eliz@gnu.org>]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: 74091-done@debbugs.gnu.org
> Date: Thu, 31 Oct 2024 12:09:20 +0100
> 
> Invoking this command twice in a row in subr.el deactivates the region,
> while the same without the argument to kill-all-local-variables keeps
> the region active.
> 
> So the problem seems to be in a lower level than string-pixel-width...

Killing local variables makes the global value of deactivate-mark be
in effect when the command loop decides whether to deactivate the
region after a command finishes.

> As I'm sure you know, applying a crude fix without fully understanding
> the problem is likely to hide other subtle bugs that may then be harder
> to investigate.

That's not a crude fix, that's what the ELisp manual tells us to do:

 -- Variable: deactivate-mark
     If an editor command sets this variable non-‘nil’, then the editor
     command loop deactivates the mark after the command returns (if
     Transient Mark mode is enabled).  All the primitives that change
     the buffer set ‘deactivate-mark’, to deactivate the mark when the
     command is finished.  Setting this variable makes it buffer-local.

     To write Lisp code that modifies the buffer without causing
     deactivation of the mark at the end of the command, bind
     ‘deactivate-mark’ to ‘nil’ around the code that does the
     modification.  For example:

          (let (deactivate-mark)
            (insert " "))

> >> And in both the old
> >> implementation and in the new one, the modification is in a different
> >> buffer, is that expected to disable the mark in the original buffer?
> >
> > The variable deactivate-mark only becomes buffer-local if set;
> > otherwise the global value will be changed.
> 
> Could you perhaps elaborate?  I see that running a command that modifies
> a different buffer does not deactivate the region in the current buffer,
> which is basically what I would expect.

You are asking me to elaborate about what? about the local value of
deactivate-mark or about why you see what you see (in a scenario you
haven't described)?

Look, you are welcome to keep debugging this if you are interested.  I
invested enough of my time into figuring out why the region was
deactivated by C-n, and the solution I installed satisfies me.  But
you are welcome to keep digging, and let me tell you what I found to
save you some non-trivial tinkering:

 . The region is deactivated because of this in our command loop:

	  if (!NILP (Vdeactivate_mark))
	    /* If `select-active-regions' is non-nil, this call to
	       `deactivate-mark' also sets the PRIMARY selection.  */
	    call0 (Qdeactivate_mark);

   This is consistent with what the ELisp manual says, see the above
   citation.

 . Deactivate-mark is set non-nil by the low-level subroutines that
   modify the buffer, again according to the manual.  Two such
   buffer-modification calls were present in string-pixel-width: one
   in string-pixel-width itself, when it inserts the string into a
   work buffer, the other in work-buffer--release where it erases the
   work buffer.  This happens in in prepare_to_modify_buffer_1:

    signal_before_change (start, end, preserve_ptr);
    Fset (Qdeactivate_mark, Qt);

 . Here is a Lisp-level backtrace from one call which sets
   deactivate-mark in your recipe, as collected by GDB:

    Lisp Backtrace:
    "string-pixel-width" (0x65a8940)
    "progn" (0x65a8c40)
    "eval" (0x65a8f90)
    "posn-at-point" (0xa084200)
    "line-move-visual" (0xa084170)
    "line-move" (0xa084108)
    "next-line" (0x65aea10)
    "funcall-interactively" (0x65aea08)
    "call-interactively" (0xa084078)
    "command-execute" (0x65af798)

   As you see, next-line calls posn-at-point, which formats the mode
   line (to calculate is height), and that invokes the :eval form.
   Here's the C backtrace which captures the details missing from the
   above Lisp backtrace:

   Thread 1 hit Hardware watchpoint 4: Vdeactivate_mark

   Old value = XIL(0)
   New value = XIL(0x30)
   0x00bd2da5 in store_symval_forwarding (valcontents=..., newval=XIL(0x30),
       buf=0x8f4c48) at data.c:1430
   1430          *XOBJFWD (valcontents)->objvar = newval;
   (gdb) bt
   #0  0x00bd2da5 in store_symval_forwarding (valcontents=..., newval=XIL(0x30),
       buf=0x8f4c48) at data.c:1430
   #1  0x00bd3da3 in set_internal (symbol=XIL(0x6210), newval=XIL(0x30),
       where=XIL(0xa0000000008f4c48), bindflag=SET_INTERNAL_SET) at data.c:1759
   #2  0x00bd37c5 in Fset (symbol=XIL(0x6210), newval=XIL(0x30)) at data.c:1630
   #3  0x00b5b757 in prepare_to_modify_buffer_1 (start=1, end=1, preserve_ptr=0x0)
       at insdel.c:2073
   #4  0x00b5b784 in prepare_to_modify_buffer (start=1, end=1, preserve_ptr=0x0)
       at insdel.c:2083
   #5  0x00b57e1b in insert_from_string_1 (string=XIL(0x800000000bc07ad8), pos=0,
       pos_byte=0, nchars=3, nbytes=3, inherit=false, before_markers=false)
       at insdel.c:1023
   #6  0x00b57c15 in insert_from_string (string=XIL(0x800000000bc07ad8), pos=0,
       pos_byte=0, length=3, length_byte=3, inherit=false) at insdel.c:974
   #7  0x00be5dbf in general_insert_function (insert_func=0xb57249 <insert>,
       insert_from_string_func=0xb57b92 <insert_from_string>, inherit=false,
       nargs=1, args=0xa084250) at editfns.c:1336
   #8  0x00be5e59 in Finsert (nargs=1, args=0xa084250) at editfns.c:1372
   #9  0x00c71edc in exec_byte_code (fun=XIL(0xa0000000008f45b0),
       args_template=513, nargs=1, args=0x65a8948) at bytecode.c:1417
   #10 0x00c019a3 in funcall_lambda (fun=XIL(0xa0000000008f45b0), nargs=1,
       arg_vector=0x65a8940) at eval.c:3238
   #11 0x00c017be in apply_lambda (fun=XIL(0xa0000000008f45b0),
       args=XIL(0xc00000000071c2d0), count=640) at eval.c:3201
   #12 0x00bff56b in eval_sub (form=XIL(0xc00000000071c2e0)) at eval.c:2631
   #13 0x00bf7cde in Fprogn (body=XIL(0xc00000000071c2b0)) at eval.c:439
   #14 0x00bfec17 in eval_sub (form=XIL(0xc00000000071c2f0)) at eval.c:2535
   #15 0x00bfe644 in Feval (form=XIL(0xc00000000071c2f0), lexical=XIL(0x30))
       at eval.c:2448
   #16 0x00c010df in funcall_subr (subr=0x128a1c0 <Seval>, numargs=2,
       args=0x65a8f90) at eval.c:3149
   #17 0x00c00a02 in funcall_general (fun=XIL(0xa00000000128a1c0), numargs=2,
       args=0x65a8f90) at eval.c:3026
   #18 0x00c00d92 in Ffuncall (nargs=3, args=0x65a8f88) at eval.c:3079
   #19 0x00bfbd5f in internal_condition_case_n (bfun=0xc00c46 <Ffuncall>,
       nargs=3, args=0x65a8f88, handlers=XIL(0x30),
       hfun=0x9dfc51 <dsafe_eval_handler>) at eval.c:1687
   #20 0x009dfd7d in dsafe__call (inhibit_quit=true, f=0xc00c46 <Ffuncall>,
       nargs=3, args=0x65a8f88) at xdisp.c:3093
   #21 0x009dfef9 in dsafe_eval (sexpr=XIL(0xc00000000071c2f0)) at xdisp.c:3129
   #22 0x00a3033e in display_mode_element (it=0x65a93d0, depth=2, field_width=0,
       precision=0, elt=XIL(0xc00000000071c300), props=XIL(0), risky=false)
       at xdisp.c:28039
   #23 0x00a308f2 in display_mode_element (it=0x65a93d0, depth=1, field_width=0,
       precision=0, elt=XIL(0xc00000000071c290), props=XIL(0), risky=false)
       at xdisp.c:28125
   #24 0x00a2e976 in display_mode_line (w=0xbb0b428,
       face_id=MODE_LINE_ACTIVE_FACE_ID, format=XIL(0xc00000000071c310))
       at xdisp.c:27550
   #25 0x009d54b2 in pos_visible_p (w=0xbb0b428, charpos=1, x=0x65adfec,
       y=0x65adfe8, rtop=0x65adffc, rbot=0x65adff8, rowh=0x65adff4,
       vpos=0x65adff0) at xdisp.c:1732
   #26 0x00a65e8e in Fpos_visible_in_window_p (pos=XIL(0),
       window=XIL(0xa00000000bb0b428), partially=XIL(0x30)) at window.c:2018
   #27 0x00b2810d in Fposn_at_point (pos=XIL(0), window=XIL(0xa00000000bb0b428))
       at keyboard.c:12552

   Translation: posn-at-point called pos-visible-in-window-p, , which
   called pos_visible_p, which called display_mode_line.  That
   eventually called the :eval form, and inserted the string via
   insert_from_string_1, which called prepare_to_modify_buffer_1,
   which set deactivate-mark to t.

That's what I saw and what led me to my solution, according to what
the ELisp manual says.