bug#61091: 30.0.50; y-or-n-p clobbers match data

bug#61091: 30.0.50; y-or-n-p clobbers match data

[Richard Stallman]

I find in some of my personal code that calls to y-or-n-p
clobber the match data.  That code worked ok when I first wrote it,
and for several years after that.  Putting (save-match-data...)
around the call to y-or-n-p seems to fix it.

Did some recent change cause y-or-n-p to do searches
and not save the match data?

I will work up a test case for you if necessary, but that
will be a pain.  I hope with this info that work can be avoided.



In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 2.24.32, cairo version 1.16.0) of 2022-12-28 built on freetop
Repository revision: efc44727daaee4d3f9aeb19864074472e99b296a
Repository branch: master
System Description: Trisquel GNU/Linux Nabia (10.0)

Configured using:
 'configure x86_64-linux-gnu 'CFLAGS--O0 -g'
 --with-gnutls=ifavailable'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GPM GSETTINGS HARFBUZZ JPEG LIBOTF
LIBSELINUX LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XINPUT2
XPM GTK2 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: RMAIL

Minor modes in effect:
  shell-dirtrack-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: linux
  auto-encryption-mode: t
  auto-compression-mode: t
  abbrev-mode: t

Load-path shadows:
None found.

Features:
(shadow emacsbug cus-start cus-load compare-w warnings icons shortdoc
log-edit add-log vc smerge-mode whitespace diff vc-git diff-mode
vc-dispatcher bug-reference sh-script rx executable jka-compr rng-xsd
xsd-regexp rng-cmpct rng-nxml rng-valid rng-loc rng-uri rng-parse
nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns nxml-mode
nxml-outln nxml-rap nxml-util nxml-enc xmltok parse-time iso8601
vc-cvs vc-rcs log-view easy-mmode pcvs-util mhtml-mode css-mode smie
eww xdg url-queue mm-url gnus nnheader range wid-edit color js treesit
imenu cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align
cc-engine cc-vars cc-defs sgml-mode facemenu mule-util
display-line-numbers novice cl-extra edmacro kmacro format-spec
help-fns radix-tree cl-print debug backtrace find-func rect quail
help-mode ispell rmailkwd dabbrev misearch multi-isearch epa-mail
shell pcomplete thingatpt files-x grep compile comint ansi-osc
ansi-color ring rmailout textsec uni-scripts idna-mapping
ucs-normalize uni-confusable textsec-check shr pixel-fill kinsoku
url-file svg xml dom mailalias qp rmailmm message sendmail yank-media
puny rfc822 mml mml-sec epa epg rfc6068 epg-config gnus-util
text-property-search time-date mm-decode mm-bodies mm-encode
mailabbrev gmm-utils mailheader mail-parse rfc2231 dired-aux dired
dired-loaddefs term/linux view derived disp-table advice rmailsum
rmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
finder-inf package browse-url url url-proxy url-privacy url-expand
url-methods url-history url-cookie generate-lisp-file url-domsuf
url-util mailcap url-handlers url-parse auth-source cl-seq eieio
eieio-core cl-macs password-cache json subr-x map byte-opt gv bytecomp
byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl tooltip cconv
eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice
seq simple cl-generic indonesian philippine cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese
eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese composite emoji-zwj charscript charprop
case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable
backquote threads dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x
multi-tty make-network-process emacs)

Memory information:
((conses 16 870629 121082)
 (symbols 48 20781 23)
 (strings 32 109387 13559)
 (string-bytes 1 2505004)
 (vectors 16 70750)
 (vector-slots 8 1692165 149404)
 (floats 8 248 410)
 (intervals 56 99582 2823)
 (buffers 976 110))
[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]


-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#61091: 30.0.50; y-or-n-p clobbers match data

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Thu, 26 Jan 2023 23:05:10 -0500
> 
> I find in some of my personal code that calls to y-or-n-p
> clobber the match data.  That code worked ok when I first wrote it,
> and for several years after that.  Putting (save-match-data...)
> around the call to y-or-n-p seems to fix it.
> 
> Did some recent change cause y-or-n-p to do searches
> and not save the match data?

y-or-n-p originally was implemented in C and was relatively simple.
Since then it was moved to Lisp and became a monster, see subr.el.  It
is anyone's guess where in that code we clobber the match data.  Given
a test case, we could debug and find what does this, but is it really
worth our while?  In general, Lisp programs should not rely on any
function not to clobber match data, unless that function is documented
to preserve match data.

bug#61091: 30.0.50; y-or-n-p clobbers match data

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> y-or-n-p originally was implemented in C and was relatively simple.
> Since then it was moved to Lisp and became a monster, see subr.el.  It
> is anyone's guess where in that code we clobber the match data.

I see that the code calls `substitute-command-keys' which potentially
changes match data.

Michael.

bug#61091: 30.0.50; y-or-n-p clobbers match data

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: rms@gnu.org,  61091@debbugs.gnu.org
> Date: Fri, 27 Jan 2023 15:38:14 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > y-or-n-p originally was implemented in C and was relatively simple.
> > Since then it was moved to Lisp and became a monster, see subr.el.  It
> > is anyone's guess where in that code we clobber the match data.
> 
> I see that the code calls `substitute-command-keys' which potentially
> changes match data.

Yes, that's also my prime suspect.

bug#61091: 30.0.50; y-or-n-p clobbers match data

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  >   In general, Lisp programs should not rely on any
  > function not to clobber match data, unless that function is documented
  > to preserve match data.

Allowance should be made for the fact that `y-or-n-p'
was safe for the match data for over 35 years.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#61091: 30.0.50; y-or-n-p clobbers match data

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: 61091@debbugs.gnu.org
> Date: Sun, 29 Jan 2023 00:18:24 -0500
> 
>   >   In general, Lisp programs should not rely on any
>   > function not to clobber match data, unless that function is documented
>   > to preserve match data.
> 
> Allowance should be made for the fact that `y-or-n-p'
> was safe for the match data for over 35 years.

I don't object if it's easy to do so.  But we decided long ago that
significant efforts for that purpose are not worth our while, and even
documented this in the ELisp reference manual.

bug#61091: 30.0.50; y-or-n-p clobbers match data

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > Allowance should be made for the fact that `y-or-n-p'
  > > was safe for the match data for over 35 years.

  > I don't object if it's easy to do so.

I think it should be easy.  How about trying it?

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#61091: 30.0.50; y-or-n-p clobbers match data

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: 61091@debbugs.gnu.org
> Date: Sat, 04 Feb 2023 00:15:24 -0500
> 
>   > > Allowance should be made for the fact that `y-or-n-p'
>   > > was safe for the match data for over 35 years.
> 
>   > I don't object if it's easy to do so.
> 
> I think it should be easy.  How about trying it?

Done with the patch below.

diff --git a/lisp/subr.el b/lisp/subr.el
index 0f754fc..32c9974 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3573,12 +3573,14 @@ y-or-n-p
 			    (if (or (zerop l) (eq ?\s (aref prompt (1- l))))
 				"" " ")
 			    (if dialog ""
-                              (substitute-command-keys
-                               (if help-form
-                                   (format "(\\`y', \\`n' or \\`%s') "
-                                           (key-description
-                                            (vector help-char)))
-                                 "(\\`y' or \\`n') ")))))))
+                              ;; Don't clobber caller's match data.
+                              (save-match-data
+                                (substitute-command-keys
+                                 (if help-form
+                                     (format "(\\`y', \\`n' or \\`%s') "
+                                             (key-description
+                                              (vector help-char)))
+                                   "(\\`y' or \\`n') "))))))))
         ;; Preserve the actual command that eventually called
         ;; `y-or-n-p' (otherwise `repeat' will be repeating
         ;; `exit-minibuffer').