GNU bug report logs - #41445
26.3; Query-replace triggers "match data clobbered by..."

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 41445 in the body.
You can then email your comments to 41445 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: ture <at> turepalsson.se (Ture Pålsson)
To: bug-gnu-emacs <at> gnu.org
Cc: ture <at> turepalsson.se
Subject: 26.3; Query-replace triggers "match data clobbered by..."
Date: Fri, 22 May 2020 07:07:24 +0200

1. Run macOS (Catalina, in my case, but I don't think it matters.)

2. Have a folder named "ä" (that's a single character, U+00E4)
   and a file in it.

3. Visit that file.

4. While the file is not modified, do a query-replace that will replace
   something.

5. When Emacs asks about the first replacement, type 'y'.

==> You get an error message, "Match data clobbered by buffer
modification hooks".

Setting before-change-functions, after-change-functions, and
first-change-hook all to nil does not make the problem go away.

However, setting inhibit-modification-hooks to t *does* make it go away.

Running Emacs in a debugger, I notice that the first modification of the
buffer calls lock_file, which calls code_convert_string which, through a
long series of Emacs Lisp calls end up calling re-search-forward. I have
not tried to unravel this call chain, but:

The function ucs-normalize-region calls re-search-forward. If I wrap
that call in save-match-data, the problem goes away!


In GNU Emacs 26.3 (build 1, x86_64-apple-darwin18.2.0, NS appkit-1671.20 Version 10.14.3 (Build 18D109))
 of 2019-09-02 built on builder10-14.porkrind.org
Windowing system distributor 'Apple', version 10.3.1894
Recent messages:
Mark saved where search started
Mark set


Mark saved where search started
Mark set
Mark saved where search started
Making completion list...
Quit
Auto-saving...
Saving file /Users/ture/Desktop/emacs/lisp/international/ucs-normalize.el...
Wrote /Users/ture/Desktop/emacs/lisp/international/ucs-normalize.el
Quit
Configured using:
 'configure --with-ns '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp' --with-modules'

Configured features:
NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS MODULES THREADS

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

Major mode: Emacs-Lisp

Minor modes in effect:
  shell-dirtrack-mode: t
  bug-reference-prog-mode: t
  diff-auto-refine-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  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
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny format-spec rfc822 mml
mml-sec epa derived epg gnus-util rmail rmail-loaddefs mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
completion pulse find-dired semantic/fw xref project dired-aux dired
dired-loaddefs ibuf-ext ibuffer ibuffer-loaddefs loadhist mode-local
find-func apropos shell pcomplete grep compile comint ring bug-reference
map cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align
cc-engine cc-vars cc-defs pp cl-print thingatpt help-fns radix-tree
jka-compr info misearch multi-isearch vc-git diff-mode easy-mmode view
elec-pair ansi-color iso-transl cl-extra help-mode cl finder-inf package
easymenu epg-config url-handlers url-parse auth-source cl-seq eieio
eieio-core cl-macs eieio-loaddefs password-cache url-vars seq byte-opt
gv bytecomp byte-compile cconv cl-loaddefs cl-lib time-date tooltip
eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel
term/ns-win ns-win ucs-normalize mule-util term/common-win tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic 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 charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads kqueue cocoa ns
multi-tty make-network-process emacs)

Memory information:
((conses 16 411912 23871)
 (symbols 48 28246 5)
 (miscs 40 341 2088)
 (strings 32 61990 7915)
 (string-bytes 1 1687293)
 (vectors 16 52073)
 (vector-slots 8 1712291 139060)
 (floats 8 71 567)
 (intervals 56 32656 600)
 (buffers 992 37))

Message #8 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Ture Pålsson <ture <at> turepalsson.se>
Cc: 41445 <at> debbugs.gnu.org
Subject: bug#41445: 26.3; Query-replace triggers "match data clobbered by..." 
Date: Fri, 22 May 2020 12:46:03 +0200

[Message part 1 (text/plain, inline)]

tags 41445 patch
stop

Thank you! Clearly nobody expects normalisation functions to clobber match data.

[0001-Don-t-clobber-match-data-in-Unicode-normalisation-bu.patch (application/octet-stream, attachment)]

Message #13 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3;
 Query-replace triggers "match data clobbered by..."
Date: Fri, 22 May 2020 14:11:44 +0300

> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Fri, 22 May 2020 12:46:03 +0200
> Cc: 41445 <at> debbugs.gnu.org
> 
> Thank you! Clearly nobody expects normalisation functions to clobber match data.

Is that the right place to save-match-data, though?  Should we perhaps
do that where utf-8-hfs file names are encoded?

Thanks.

Message #16 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..."
Date: Fri, 22 May 2020 13:16:04 +0200

22 maj 2020 kl. 13.11 skrev Eli Zaretskii <eliz <at> gnu.org>:

> Is that the right place to save-match-data, though?  Should we perhaps
> do that where utf-8-hfs file names are encoded?

What location did you have in mind, more precisely?

Message #19 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..."
Date: Fri, 22 May 2020 15:07:24 +0300

> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Fri, 22 May 2020 13:16:04 +0200
> Cc: ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
> 
> 22 maj 2020 kl. 13.11 skrev Eli Zaretskii <eliz <at> gnu.org>:
> 
> > Is that the right place to save-match-data, though?  Should we perhaps
> > do that where utf-8-hfs file names are encoded?
> 
> What location did you have in mind, more precisely?

I think in ucs-normalize-hfs-nfd-post-read-conversion and
ucs-normalize-hfs-nfd-pre-write-conversion.  IOW, so that this only
affects encoding and decoding macOS file names.  WDYT?

Message #22 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Ture Pålsson <ture <at> turepalsson.se>, 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..."
Date: Fri, 22 May 2020 14:21:04 +0200

22 maj 2020 kl. 14.07 skrev Eli Zaretskii <eliz <at> gnu.org>:

> I think in ucs-normalize-hfs-nfd-post-read-conversion and
> ucs-normalize-hfs-nfd-pre-write-conversion.  IOW, so that this only
> affects encoding and decoding macOS file names.  WDYT?

Both of these call ucs-normalize-region, which makes it natural to save match data in that function. Conversely, anything that calls ucs-normalize-region would have to be wrapped, not just the two functions you mentioned. In other words, there seems to be no advantage in saving the match data in those functions, only disadvantages.

Message #25 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..."
Date: Fri, 22 May 2020 15:35:38 +0300

> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Fri, 22 May 2020 14:21:04 +0200
> Cc: Ture Pålsson <ture <at> turepalsson.se>, 41445 <at> debbugs.gnu.org
> 
> 22 maj 2020 kl. 14.07 skrev Eli Zaretskii <eliz <at> gnu.org>:
> 
> > I think in ucs-normalize-hfs-nfd-post-read-conversion and
> > ucs-normalize-hfs-nfd-pre-write-conversion.  IOW, so that this only
> > affects encoding and decoding macOS file names.  WDYT?
> 
> Both of these call ucs-normalize-region, which makes it natural to save match data in that function. Conversely, anything that calls ucs-normalize-region would have to be wrapped, not just the two functions you mentioned. In other words, there seems to be no advantage in saving the match data in those functions, only disadvantages.

My line of reasoning was that only the callers of ENCODE_FILE and
DECODE_FILE will not expect the match-data to be clobbered.  Code that
calls ucs-normalize-region directly may or may not be bothered by the
clobbering, so we should leave that to the caller.

The advantage of not doing this unconditionally is that we don't
unnecessarily punishing callers that don't need match-data to be
saved.

Message #28 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..." 
Date: Sat, 23 May 2020 13:36:37 +0200

> My line of reasoning was that only the callers of ENCODE_FILE and
> DECODE_FILE will not expect the match-data to be clobbered.  Code that
> calls ucs-normalize-region directly may or may not be bothered by the
> clobbering, so we should leave that to the caller.
> 
> The advantage of not doing this unconditionally is that we don't
> unnecessarily punishing callers that don't need match-data to be
> saved.

For callers of the ucs-normalize- functions, correctness should come first; their names, semantics or descriptions do not lead the user to suspect them of clobbering the match data. It is an implementation leakage which can be quite unexpected, as is witnessed by this bug.

For example, in one of the few calls I could find at all, this might be classified as a near-miss, only saved by the left-to-right argument evaluation order:


                (lookup-new-entry 
                 'regular dictionary (match-string 0)
                 (ucs-normalize-HFS-NFC-string (match-string 1))))

Message #31 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca> 
Cc: ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..."
Date: Sat, 23 May 2020 15:28:42 +0300

> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Sat, 23 May 2020 13:36:37 +0200
> Cc: ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
> 
> > My line of reasoning was that only the callers of ENCODE_FILE and
> > DECODE_FILE will not expect the match-data to be clobbered.  Code that
> > calls ucs-normalize-region directly may or may not be bothered by the
> > clobbering, so we should leave that to the caller.
> > 
> > The advantage of not doing this unconditionally is that we don't
> > unnecessarily punishing callers that don't need match-data to be
> > saved.
> 
> For callers of the ucs-normalize- functions, correctness should come first; their names, semantics or descriptions do not lead the user to suspect them of clobbering the match data. It is an implementation leakage which can be quite unexpected, as is witnessed by this bug.

I thought we had some advice to Lisp programs not to assume that
match-data will be preserved, but maybe I'm misremembering.  Stefan,
do you remember something along these lines?

Anyway, if you feel strongly about doing this on the lowest level, I
won't fight.

Message #34 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Mattias Engdegård <mattiase <at> acm.org>, ture <at> turepalsson.se,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3;
 Query-replace triggers "match data clobbered by..."
Date: Sat, 23 May 2020 14:37:17 +0200

Am Sa., 23. Mai 2020 um 14:29 Uhr schrieb Eli Zaretskii <eliz <at> gnu.org>:
>
> > From: Mattias Engdegård <mattiase <at> acm.org>
> > Date: Sat, 23 May 2020 13:36:37 +0200
> > Cc: ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
> >
> > > My line of reasoning was that only the callers of ENCODE_FILE and
> > > DECODE_FILE will not expect the match-data to be clobbered.  Code that
> > > calls ucs-normalize-region directly may or may not be bothered by the
> > > clobbering, so we should leave that to the caller.
> > >
> > > The advantage of not doing this unconditionally is that we don't
> > > unnecessarily punishing callers that don't need match-data to be
> > > saved.
> >
> > For callers of the ucs-normalize- functions, correctness should come first; their names, semantics or descriptions do not lead the user to suspect them of clobbering the match data. It is an implementation leakage which can be quite unexpected, as is witnessed by this bug.
>
> I thought we had some advice to Lisp programs not to assume that
> match-data will be preserved, but maybe I'm misremembering.  Stefan,
> do you remember something along these lines?

That's at least what the manual says
(https://www.gnu.org/software/emacs/manual/html_node/elisp/Match-Data.html):
"Notice that all functions are allowed to overwrite the match data
unless they're explicitly documented not to do so."
(Not that I like that statement, but it is current reality.)

Message #37 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: mattiase <at> acm.org, ture <at> turepalsson.se, monnier <at> iro.umontreal.ca,
 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3;
 Query-replace triggers "match data clobbered by..."
Date: Sat, 23 May 2020 16:07:05 +0300

> From: Philipp Stephani <p.stephani2 <at> gmail.com>
> Date: Sat, 23 May 2020 14:37:17 +0200
> Cc: Mattias Engdegård <mattiase <at> acm.org>, 
> 	Stefan Monnier <monnier <at> iro.umontreal.ca>, ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
> 
> > I thought we had some advice to Lisp programs not to assume that
> > match-data will be preserved, but maybe I'm misremembering.  Stefan,
> > do you remember something along these lines?
> 
> That's at least what the manual says
> (https://www.gnu.org/software/emacs/manual/html_node/elisp/Match-Data.html):
> "Notice that all functions are allowed to overwrite the match data
> unless they're explicitly documented not to do so."
> (Not that I like that statement, but it is current reality.)

Right, thanks.  I missed that.

Message #40 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, ture <at> turepalsson.se,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..." 
Date: Sat, 23 May 2020 15:08:21 +0200

23 maj 2020 kl. 14.37 skrev Philipp Stephani <p.stephani2 <at> gmail.com>:

> "Notice that all functions are allowed to overwrite the match data
> unless they're explicitly documented not to do so."
> (Not that I like that statement, but it is current reality.)

Thanks for the reference. Nevertheless, functions do use save-match-data for the benefit of their callers every now and then. The practice is fairly widespread, more so for functions that are otherwise side-effect-free. It's a matter of reasonable expectation, not following the manual to the letter.

For that matter, there are few functions explicitly documented not to clobber the match data, not counting the automatically inserted statement for functions marked pure or side-effect-free.

Message #43 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Mattias Engdegård <mattiase <at> acm.org>,
 ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..."
Date: Sat, 23 May 2020 09:36:03 -0400

>> > The advantage of not doing this unconditionally is that we don't
>> > unnecessarily punishing callers that don't need match-data to be
>> > saved.
>> 
>> For callers of the ucs-normalize- functions, correctness should come
>> first; their names, semantics or descriptions do not lead the user to
>> suspect them of clobbering the match data. It is an implementation leakage
>> which can be quite unexpected, as is witnessed by this bug.
>
> I thought we had some advice to Lisp programs not to assume that
> match-data will be preserved, but maybe I'm misremembering.  Stefan,
> do you remember something along these lines?

Right, we follow a convention where, by default, any function can
clobber the match-data, with just some exceptions (typically,
small/trivial functions).

From that point of view, I see no reason why ucs-normalize-* should be
careful to preserve the match data.

This said, *if* it is the case that many/most calls to a given function
need to preserve the match-data around calls to it, it's of course
OK to simply move the match-data-saving into that function, especially
if that function's work dwarfs that of save-match-data.

I just added the following to `save-match-data`'s docstring, to try and
clarify:

    NOTE: The convention in Elisp is that any function, except for a few
    exceptions like car/assoc/+/goto-char, can clobber the match data,
    so `save-match-data' should normally be used to save *your* match
    data rather than your caller's match data.


-- Stefan

Message #46 received at 41445 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>
Cc: Mattias Engdegård <mattiase <at> acm.org>,
 ture <at> turepalsson.se, 41445 <at> debbugs.gnu.org
Subject: RE: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..."
Date: Sat, 23 May 2020 08:43:33 -0700 (PDT)

>     NOTE: The convention in Elisp is that any function, except for a
>     few exceptions like car/assoc/+/goto-char, can clobber the match
>     data, so `save-match-data' should normally be used to save *your* 
>     match data rather than your caller's match data.

+1.  I like that, especially the last phrase.
Clear; important to get across.

Message #51 received at 41445-done <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 Ture Pålsson <ture <at> turepalsson.se>,
 41445-done <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..." 
Date: Wed, 27 May 2020 16:31:42 +0200

23 maj 2020 kl. 15.36 skrev Stefan Monnier <monnier <at> iro.umontreal.ca>:

> From that point of view, I see no reason why ucs-normalize-* should be
> careful to preserve the match data.

Very well, since there appears to be a consensus for that view, I've pushed a change to the utf-8-hfs coding only.

Message #54 received at 41445-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: ture <at> turepalsson.se, monnier <at> iro.umontreal.ca, 41445-done <at> debbugs.gnu.org
Subject: Re: bug#41445: 26.3; Query-replace triggers "match data clobbered
 by..."
Date: Wed, 27 May 2020 18:54:51 +0300

> Feedback-ID:mattiase <at> acm.or
> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Wed, 27 May 2020 16:31:42 +0200
> Cc: Eli Zaretskii <eliz <at> gnu.org>,
>         Ture Pålsson <ture <at> turepalsson.se>,
>         41445-done <at> debbugs.gnu.org
> 
> 23 maj 2020 kl. 15.36 skrev Stefan Monnier <monnier <at> iro.umontreal.ca>:
> 
> > From that point of view, I see no reason why ucs-normalize-* should be
> > careful to preserve the match data.
> 
> Very well, since there appears to be a consensus for that view, I've pushed a change to the utf-8-hfs coding only.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.