From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: npostavs@users.sourceforge.net Newsgroups: gmane.emacs.bugs Subject: bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template =?UTF-8?Q?=E2=80=98g=E2=80=99:?= Match data clobbered by buffer modification hooks) Date: Wed, 20 Jul 2016 20:56:28 -0400 Message-ID: <87bn1rdd1f.fsf@users.sourceforge.net> References: <87vb066ejv.fsf@linaro.org> <87bn1yyaui.fsf@linaro.org> <87mvlhmv0x.fsf_-_@moondust.awandering> <837fcl5zs9.fsf@gnu.org> <87a8hgkwcb.fsf@linaro.org> <8360s42mcb.fsf@gnu.org> <87eg6rgmlg.fsf@gmail.com> <83lh0y24y6.fsf@gnu.org> <83eg6q1hbo.fsf@gnu.org> <83a8hd1vzi.fsf@gnu.org> <834m7l1u8u.fsf@gnu.org> <83shv4z7e8.fsf@gnu.org> <83inw0yw9q.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1469062658 20095 80.91.229.3 (21 Jul 2016 00:57:38 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 21 Jul 2016 00:57:38 +0000 (UTC) Cc: nljlistbox2@gmail.com, jwiegley@gmail.com, rpluim@gmail.com, 23917@debbugs.gnu.org, alex.bennee@linaro.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Jul 21 02:57:22 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1bQ2Ik-0004Le-2j for geb-bug-gnu-emacs@m.gmane.org; Thu, 21 Jul 2016 02:57:22 +0200 Original-Received: from localhost ([::1]:37646 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQ2Ig-0007bb-1o for geb-bug-gnu-emacs@m.gmane.org; Wed, 20 Jul 2016 20:57:18 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:47154) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQ2IW-0007ZI-IY for bug-gnu-emacs@gnu.org; Wed, 20 Jul 2016 20:57:10 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQ2IQ-0006VH-94 for bug-gnu-emacs@gnu.org; Wed, 20 Jul 2016 20:57:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:46762) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQ2IQ-0006VB-5D; Wed, 20 Jul 2016 20:57:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bQ2IP-0004KK-QO; Wed, 20 Jul 2016 20:57:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: npostavs@users.sourceforge.net Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org, emacs-orgmode@gnu.org Resent-Date: Thu, 21 Jul 2016 00:57:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 23917 X-GNU-PR-Package: emacs,org-mode X-GNU-PR-Keywords: Original-Received: via spool by 23917-submit@debbugs.gnu.org id=B23917.146906259516594 (code B ref 23917); Thu, 21 Jul 2016 00:57:01 +0000 Original-Received: (at 23917) by debbugs.gnu.org; 21 Jul 2016 00:56:35 +0000 Original-Received: from localhost ([127.0.0.1]:59099 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bQ2Hz-0004Ja-8q for submit@debbugs.gnu.org; Wed, 20 Jul 2016 20:56:35 -0400 Original-Received: from mail-it0-f45.google.com ([209.85.214.45]:36975) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bQ2Hx-0004JN-7x for 23917@debbugs.gnu.org; Wed, 20 Jul 2016 20:56:33 -0400 Original-Received: by mail-it0-f45.google.com with SMTP id f6so3797074ith.0 for <23917@debbugs.gnu.org>; Wed, 20 Jul 2016 17:56:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=btcwXwfhqzakL56GdeVbyM1Dc3Amc2EInTK0x6RIJ5s=; b=dUrY1GLusWEDePIRXEtMKAqpZTirrXTMc71BbMUFR3ANTSqP/+jBpJ9a/VzzKOo93B 7oiVO0+3Lujop5J3btY0MKL4kChRjEgvKPE0ZJ9JYzb9uyY9wNB+yKDWZqSGog6vKztg jmHFNu7teypBPtzPE/H2ChtGswFWzhnhc8EKimrQ2hE4lhJgerxXy4JX0jkDeazE+Az3 WIzoNPTN3p9jzP7KDF6DgRS1noCF2VFU8gETkZRMPvqGCxaJ6jqpajltpKWqarFLB0gI KhKU7kaKuz+6MCSPYRE7mkMSPGu3kckoCRN9uHhrryPaLcsb42FGf4HL9Jl7SHIwaEWK 2VPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version; bh=btcwXwfhqzakL56GdeVbyM1Dc3Amc2EInTK0x6RIJ5s=; b=jXwS90V9x8gd8n7CgEhH9Da+j1ldrt6W+ObACzO8MniRGD0jCKc7AcWf2aoT2jG3Bc 1T9VK0bb93qZa0ErXX/66rucdRaNPMVsJ6XawDkMCB9EpWvBppu3XyfWddLl0pZTgBVm 0sQLpMjkLZqGH2CZckKEYjyVNeMpp75yA9ucBR4tDTvkpxr3Y81e3oNToFHTTdV3s7Oh W8603OlKZrU4mQccmS5XXnCAzRRR1hlP5RMEGTzvGwQpuUbCwUBu7xjOmxB2nT4ALUIH w6EnFMqy7LcwiYzsPgiZDs/3tRY6zjvSsxwI6hRLqNSiAPTDLk8ODEQTnXE4JE1619jk zGeA== X-Gm-Message-State: ALyK8tIRQiccIyUsIdT3LYpx1S8ymy+Xfc7f7JThGNV9GETIo9lS4ROiN760s7xsUDjPHQ== X-Received: by 10.36.64.5 with SMTP id n5mr10546657ita.78.1469062587682; Wed, 20 Jul 2016 17:56:27 -0700 (PDT) Original-Received: from zony (206-188-64-44.cpe.distributel.net. [206.188.64.44]) by smtp.googlemail.com with ESMTPSA id b89sm2566469iod.30.2016.07.20.17.56.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Jul 2016 17:56:26 -0700 (PDT) In-Reply-To: (Stefan Monnier's message of "Wed, 20 Jul 2016 16:54:03 -0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.93 (gnu/linux) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:121346 Archived-At: --=-=-= Content-Type: text/plain Stefan Monnier writes: >> Maybe there's a misunderstanding. What Noam suggested was just to >> move the code which adjusts search_regs.start[i] and .end[i] to before >> the call to replace_range. > > Oh, right, that's also an option. It might suffer from another problem, > which is that the match-data will be broken while the > before-change-functions are run, so if there's a save-match-data there > we're back to square one. Solution: adjust in between the before and after change functions. Patch below. I think there shouldn't be performance problems, although it does add yet another flag to replace_range (by the way, I noticed that the MARKERS flags is never 0, so it's redundant). I tested with the attached bug-23917-match-data-buffer-modhook.el. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=v1-0001-Adjust-match-data-before-calling-after-change-fun.patch Content-Description: patch >From a8098080dff5f83f7cbcbec2bc263f9db3b45ad9 Mon Sep 17 00:00:00 2001 From: Noam Postavsky Date: Wed, 20 Jul 2016 20:15:14 -0400 Subject: [PATCH v1] Adjust match data before calling after-change-funs * src/insdel.c (replace_range): Add new parameter ADJUST_MATCH_DATA, if true. Update all callers except Freplace_match to pass 0 for the new parameter. * src/search.c (update_search_regs): New function, extracted from Freplace_match. (Freplace_match): Remove match data adjustment code, pass 1 for ADJUST_MATCH_DATA to replace_range instead. --- src/cmds.c | 2 +- src/editfns.c | 6 +++--- src/insdel.c | 10 ++++++++-- src/lisp.h | 4 +++- src/search.c | 50 +++++++++++++++++++++++++++++--------------------- 5 files changed, 44 insertions(+), 28 deletions(-) diff --git a/src/cmds.c b/src/cmds.c index 1e44ddd..4003d8b 100644 --- a/src/cmds.c +++ b/src/cmds.c @@ -447,7 +447,7 @@ internal_self_insert (int c, EMACS_INT n) string = concat2 (string, tem); } - replace_range (PT, PT + chars_to_delete, string, 1, 1, 1); + replace_range (PT, PT + chars_to_delete, string, 1, 1, 1, 0); Fforward_char (make_number (n)); } else if (n > 1) diff --git a/src/editfns.c b/src/editfns.c index 412745d..32c8bec 100644 --- a/src/editfns.c +++ b/src/editfns.c @@ -3192,7 +3192,7 @@ DEFUN ("subst-char-in-region", Fsubst_char_in_region, /* replace_range is less efficient, because it moves the gap, but it handles combining correctly. */ replace_range (pos, pos + 1, string, - 0, 0, 1); + 0, 0, 1, 0); pos_byte_next = CHAR_TO_BYTE (pos); if (pos_byte_next > pos_byte) /* Before combining happened. We should not increment @@ -3405,7 +3405,7 @@ DEFUN ("translate-region-internal", Ftranslate_region_internal, /* This is less efficient, because it moves the gap, but it should handle multibyte characters correctly. */ string = make_multibyte_string ((char *) str, 1, str_len); - replace_range (pos, pos + 1, string, 1, 0, 1); + replace_range (pos, pos + 1, string, 1, 0, 1, 0); len = str_len; } else @@ -3446,7 +3446,7 @@ DEFUN ("translate-region-internal", Ftranslate_region_internal, { string = Fmake_string (make_number (1), val); } - replace_range (pos, pos + len, string, 1, 0, 1); + replace_range (pos, pos + len, string, 1, 0, 1, 0); pos_byte += SBYTES (string); pos += SCHARS (string); cnt += SCHARS (string); diff --git a/src/insdel.c b/src/insdel.c index 4ad1074..fc3f19f 100644 --- a/src/insdel.c +++ b/src/insdel.c @@ -1268,7 +1268,9 @@ adjust_after_insert (ptrdiff_t from, ptrdiff_t from_byte, /* Replace the text from character positions FROM to TO with NEW, If PREPARE, call prepare_to_modify_buffer. If INHERIT, the newly inserted text should inherit text properties - from the surrounding non-deleted text. */ + from the surrounding non-deleted text. + If ADJUST_MATCH_DATA, then adjust the match data before calling + signal_after_change. */ /* Note that this does not yet handle markers quite right. Also it needs to record a single undo-entry that does a replacement @@ -1279,7 +1281,8 @@ adjust_after_insert (ptrdiff_t from, ptrdiff_t from_byte, void replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new, - bool prepare, bool inherit, bool markers) + bool prepare, bool inherit, bool markers, + bool adjust_match_data) { ptrdiff_t inschars = SCHARS (new); ptrdiff_t insbytes = SBYTES (new); @@ -1426,6 +1429,9 @@ replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new, MODIFF++; CHARS_MODIFF = MODIFF; + if (adjust_match_data) + update_search_regs (from, to, from + SCHARS (new)); + signal_after_change (from, nchars_del, GPT - from); update_compositions (from, GPT, CHECK_BORDER); } diff --git a/src/lisp.h b/src/lisp.h index 6a98adb..25f811e 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3516,7 +3516,7 @@ extern void adjust_after_insert (ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t); extern void adjust_markers_for_delete (ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t); -extern void replace_range (ptrdiff_t, ptrdiff_t, Lisp_Object, bool, bool, bool); +extern void replace_range (ptrdiff_t, ptrdiff_t, Lisp_Object, bool, bool, bool, bool); extern void replace_range_2 (ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, const char *, ptrdiff_t, ptrdiff_t, bool); extern void syms_of_insdel (void); @@ -3994,6 +3994,8 @@ extern Lisp_Object make_temp_name (Lisp_Object, bool); /* Defined in search.c. */ extern void shrink_regexp_cache (void); extern void restore_search_regs (void); +extern void update_search_regs (ptrdiff_t oldstart, + ptrdiff_t oldend, ptrdiff_t newend); extern void record_unwind_save_match_data (void); struct re_registers; extern struct re_pattern_buffer *compile_pattern (Lisp_Object, diff --git a/src/search.c b/src/search.c index 5c949ad..1b82c94 100644 --- a/src/search.c +++ b/src/search.c @@ -2727,8 +2727,15 @@ DEFUN ("replace-match", Freplace_match, Sreplace_match, 1, 5, 0, /* Replace the old text with the new in the cleanest possible way. */ replace_range (search_regs.start[sub], search_regs.end[sub], - newtext, 1, 0, 1); + newtext, 1, 0, 1, 1); newpoint = search_regs.start[sub] + SCHARS (newtext); + /* Update saved data to match adjustment made by replace_range. */ + { + ptrdiff_t change = newpoint - sub_end; + if (sub_start >= sub_end) + sub_start += change; + sub_end += change; + } if (case_action == all_caps) Fupcase_region (make_number (search_regs.start[sub]), @@ -2742,26 +2749,6 @@ DEFUN ("replace-match", Freplace_match, Sreplace_match, 1, 5, 0, || search_regs.num_regs != num_regs) error ("Match data clobbered by buffer modification hooks"); - /* Adjust search data for this change. */ - { - ptrdiff_t oldend = search_regs.end[sub]; - ptrdiff_t oldstart = search_regs.start[sub]; - ptrdiff_t change = newpoint - search_regs.end[sub]; - ptrdiff_t i; - - for (i = 0; i < search_regs.num_regs; i++) - { - if (search_regs.start[i] >= oldend) - search_regs.start[i] += change; - else if (search_regs.start[i] > oldstart) - search_regs.start[i] = oldstart; - if (search_regs.end[i] >= oldend) - search_regs.end[i] += change; - else if (search_regs.end[i] > oldstart) - search_regs.end[i] = oldstart; - } - } - /* Put point back where it was in the text. */ if (opoint <= 0) TEMP_SET_PT (opoint + ZV); @@ -3102,6 +3089,27 @@ restore_search_regs (void) } } +/* Called from replace-match via replace_range. */ +void +update_search_regs (ptrdiff_t oldstart, ptrdiff_t oldend, ptrdiff_t newend) +{ + /* Adjust search data for this change. */ + ptrdiff_t change = newend - oldend; + ptrdiff_t i; + + for (i = 0; i < search_regs.num_regs; i++) + { + if (search_regs.start[i] >= oldend) + search_regs.start[i] += change; + else if (search_regs.start[i] > oldstart) + search_regs.start[i] = oldstart; + if (search_regs.end[i] >= oldend) + search_regs.end[i] += change; + else if (search_regs.end[i] > oldstart) + search_regs.end[i] = oldstart; + } +} + static void unwind_set_match_data (Lisp_Object list) { -- 2.8.0 --=-=-= Content-Type: application/emacs-lisp Content-Disposition: attachment; filename=bug-23917-match-data-buffer-modhook.el Content-Transfer-Encoding: quoted-printable Content-Description: test elisp (defun bug-23917-after-modhook (beg end length) (message "match-data-post1: %S" (mapcar #'marker-position (match-data))) (save-match-data nil) (message "match-data-post2: %S" (mapcar #'marker-position (match-data)))) (defun bug-23917-before-modhook (beg end) (message "match-data-pre-1: %S" (mapcar #'marker-position (match-data))) (save-match-data nil) (message "match-data-pre-2: %S" (mapcar #'marker-position (match-data)))) (defun bug-23917-doit () (interactive) (with-current-buffer (get-buffer-create "*bug 23917*") (switch-to-buffer (current-buffer)) (erase-buffer) (insert "xxxxxxxxx yyyyyyyyy zzzzzzzzz") (goto-char 1) (re-search-forward "\\(x+\\) \\(y+\\) \\(z+\\)") (let ((after-change-functions '(bug-23917-after-modhook)) (before-change-functions '(bug-23917-before-modhook))) (replace-match "3" t t nil 3)) (view-echo-area-messages))) (bug-23917-doit) --=-=-=--