unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Alan Mackenzie <acm@muc.de>
To: Juri Linkov <juri@linkov.net>
Cc: Jayden Navarro <jayden@yugabyte.com>, 36328@debbugs.gnu.org
Subject: bug#36328: 26.2; Args out of range on search-and-replace of *.cc file
Date: Tue, 25 Jun 2019 09:47:08 +0000	[thread overview]
Message-ID: <20190625094708.GB5471@ACM> (raw)
In-Reply-To: <87d0j27wiu.fsf@mail.linkov.net>

Hello, Juri.

On Mon, Jun 24, 2019 at 22:18:53 +0300, Juri Linkov wrote:
> Hello, Alan.

> >> I think first we should try to narrow down the source of this match
> >> data leak.

> > Is there really such a thing as a match data leak?  I don't think
> > there's any convention that the match data are preserved over large
> > bits of code, particularly when different libraries are involved.
> > There is nothing documented in the Elisp manual that I can see.

> Yes, it seems such match-data leak is considered at least undesirable.
> I remember efforts to replace string-match with string-match-p in
> potentially unsafe places and to wrap more code in save-match-data.
> But I guess such efforts are futile since this task is endless.

I think so, too.  I remembered being puzzled in my early Emacs days,
wondering whether the save-match-data should go in the function which
messes it up, or the function which cares about it.

> Usually it's enough for a function that cares about preserving
> match-data to protect it from mutation.

I now think this is the best place to put save-match-data.

> >> Then we could decide what is the best solution.  Currently I see no
> >> such place in isearch-lazy-highlight-new-loop that calls external
> >> code.

> > isearch-lazy-highlight-new-loop calls (sit-for 0), which calls
> > redisplay, which calls font locking.

> You are right that it's too much to expect that the match-data will be
> preserved after redisplay, and we can't find and fix all places that
> change match-data, so save-match-data needs be added to perform-replace
> somewhere to protect match-data.

Yes, I think so.

> Since (sit-for 0) is unsafe for match-data, the first candidate to be
> wrapped in save-match-data is (sit-for 0) itself in
> isearch-lazy-highlight-new-loop.

> But perhaps more correct would be to use save-match-data in the same
> function that cares about preserving its match-data, so the second
> candidate to use save-match-data is perform-replace.  Then the need
> of using save-match-data will be self-evident for everyone who will
> look at the code in perform-replace: here we use match-data, and here
> we protect it in the same function.

My feeling is that perform-replace (or, possibly, replace-highlight) is
the best place to put a save-match-data.

-- 
Alan Mackenzie (Nuremberg, Germany).





  reply	other threads:[~2019-06-25  9:47 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-21 23:03 bug#36328: 26.2; Args out of range on search-and-replace of *.cc file Jayden Navarro
     [not found] ` <mailman.612.1561158667.10840.bug-gnu-emacs@gnu.org>
2019-06-22 13:25   ` Alan Mackenzie
2019-06-22 14:25     ` Jayden Navarro
2019-06-22 14:51       ` Juanma Barranquero
2019-06-22 16:09         ` Jayden Navarro
2019-06-22 20:50       ` Alan Mackenzie
2019-06-22 21:27         ` Jayden Navarro
2019-06-22 22:38           ` Jayden Navarro
2019-06-22 23:02             ` Jayden Navarro
2019-06-23 12:22             ` Alan Mackenzie
2019-06-23 16:14               ` Jayden Navarro
2019-06-23 19:32                 ` Alan Mackenzie
2019-06-23 21:19                   ` Juri Linkov
2019-06-23 21:42                     ` Jayden Navarro
2019-06-24 19:05                       ` Juri Linkov
2019-06-24 20:03                         ` Jayden Navarro
2019-06-24  7:52                     ` Alan Mackenzie
2019-06-24 19:18                       ` Juri Linkov
2019-06-25  9:47                         ` Alan Mackenzie [this message]
2019-06-25 19:58                           ` Juri Linkov
2019-07-04 21:09                             ` Juri Linkov
2019-07-05  6:11                               ` Eli Zaretskii
2019-07-05 19:12                                 ` Juri Linkov
2019-10-02 23:53                                   ` Stefan Kangas
2019-06-23 20:10 ` bug#36328: [jayden@yugabyte.com: Re: bug#36328: 26.2; Args out of range on search-and-replace of *.cc file] Alan Mackenzie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190625094708.GB5471@ACM \
    --to=acm@muc.de \
    --cc=36328@debbugs.gnu.org \
    --cc=jayden@yugabyte.com \
    --cc=juri@linkov.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).