bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement

Ren Victor
I attach an ert case to show the recipe.

This issue happens when modification hooks modify the text before the
end of replacement text.

`replace-match' calls `replace_range' to do the actual replacement,
which can trigger modification hooks.  Before calling `replace_range',
the position of the end of replacement is saved. After `replace_range',
the point is moved to the saved position.

But the end of replacement might be changed inside of
`replace_range'.  So the final movement of point may end up to a wrong
place.

Other types of modification (insert or delete) do not have this issue.
`point' is adjusted before running modification hooks.

In `replace_range', the point is also relocated.  I am not sure why it has
to be moved again just before returning from `replace-match'.

This is not a new issue.  It is in Emacs24 at least.


In GNU Emacs 27.0.90 (build 1, x86_64-pc-cygwin)
 of 2020-03-04 built on moufang2
Repository revision: afff43a72e96fcccabe77ff63226cddd540e068d
Repository branch: master
Windowing system distributor 'Microsoft Corp.', version 10.0.19041
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Quit
ert
next-line: End of buffer [3 times]
test-replace-match
Ran 1 tests, 0 results were as expected, 1 unexpected
next-line: End of buffer
previous-line: Beginning of buffer [13 times]
previous-line: Beginning of buffer
Configured using:
 'configure
 --srcdir=/home/kbrown/src/cygpackages/emacs/emacs-27.0.90-1.x86_64/src/emacs-27.0.90
 --prefix=/usr --exec-prefix=/usr --localstatedir=/var --sysconfdir=/etc
 --docdir=/usr/share/doc/emacs --htmldir=/usr/share/doc/emacs/html -C
 --with-w32 'CFLAGS=-ggdb -O2 -pipe -Wall -Werror=format-security
 -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong
 --param=ssp-buffer-size=4
 -fdebug-prefix-map=/home/kbrown/src/cygpackages/emacs/emacs-27.0.90-1.x86_64/build=/usr/src/debug/emacs-27.0.90-1
 -fdebug-prefix-map=/home/kbrown/src/cygpackages/emacs/emacs-27.0.90-1.x86_64/src/emacs-27.0.90=/usr/src/debug/emacs-27.0.90-1'
 CPPFLAGS= LDFLAGS='

Configured features:
XPM JPEG TIFF GIF PNG SOUND DBUS GLIB NOTIFY GFILENOTIFY ACL GNUTLS
LIBXML2 HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS XIM MODULES THREADS JSON
PDUMPER LCMS2 GMP

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

Major mode: Emacs-Lisp

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-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
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml mml-sec password-cache epa derived epg epg-config
gnus-util rmail rmail-loaddefs text-property-search time-date subr-x
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils cl-seq cl-extra seq byte-opt bytecomp byte-compile cconv
cl-macs gv ert pp ewoc debug backtrace help-mode find-func vc-git
diff-mode easymenu easy-mmode cl-loaddefs cl-lib tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel disp-table
term/w32-win w32-win w32-vars 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 tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer 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
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 dbusbind
gfilenotify w32 lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 62659 8699)
 (symbols 48 7750 1)
 (strings 32 21627 1406)
 (string-bytes 1 668356)
 (vectors 16 12971)
 (vector-slots 8 156156 9450)
 (floats 8 40 94)
 (intervals 56 307 1)
 (buffers 1000 14))

test_replace_match.el (718 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement

Lars Ingebrigtsen
Ren Victor <[hidden email]> writes:

> In `replace_range', the point is also relocated.  I am not sure why it has
> to be moved again just before returning from `replace-match'.

I can reproduce this error on Emacs 28.

The problem seems to be this in Freplace_match?  

  /* Put point back where it was in the text, if possible.  */
  TEMP_SET_PT (clip_to_bounds (BEGV, opoint + (opoint <= 0 ? ZV : 0), ZV));
  /* Now move point "officially" to the start of the inserted replacement.  */
  move_if_not_intangible (newpoint);

Uhm...  is that comment wrong?  Aren't we moving point to the end of the
inserted replacement?

Anyway, removing that move makes the suggested test not fail, but it
leads to a bunch of other tests failing, so it's doing something right,
at least.

However, just before that, there's this:

  /* The replace_range etc. functions can trigger modification hooks
     (see signal_before_change and signal_after_change).  Try to error
     out if these hooks clobber the match data since clobbering can
     result in confusing bugs.  Although this sanity check does not
     catch all possible clobberings, it should catch many of them.  */
  if (! (search_regs.num_regs == num_regs
         && search_regs.start[sub] == newstart
         && search_regs.end[sub] == newpoint))
    error ("Match data clobbered by buffer modification hooks");

So replace_match is already signalling some errors on buffer
modification hooks, but not here.  So is this a bug or just something
that should be documented better?

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



Reply | Threaded
Open this post in threaded view
|

bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement

Eli Zaretskii
> From: Lars Ingebrigtsen <[hidden email]>
> Date: Sat, 17 Oct 2020 11:49:42 +0200
> Cc: [hidden email]
>
>   /* Put point back where it was in the text, if possible.  */
>   TEMP_SET_PT (clip_to_bounds (BEGV, opoint + (opoint <= 0 ? ZV : 0), ZV));
>   /* Now move point "officially" to the start of the inserted replacement.  */
>   move_if_not_intangible (newpoint);
>
> Uhm...  is that comment wrong?  Aren't we moving point to the end of the
> inserted replacement?

Yes, it should say "end", not "start".

>   /* The replace_range etc. functions can trigger modification hooks
>      (see signal_before_change and signal_after_change).  Try to error
>      out if these hooks clobber the match data since clobbering can
>      result in confusing bugs.  Although this sanity check does not
>      catch all possible clobberings, it should catch many of them.  */
>   if (! (search_regs.num_regs == num_regs
> && search_regs.start[sub] == newstart
> && search_regs.end[sub] == newpoint))
>     error ("Match data clobbered by buffer modification hooks");
>
> So replace_match is already signalling some errors on buffer
> modification hooks, but not here.  So is this a bug or just something
> that should be documented better?

Personally, I wonder what was expected here.  If the modification
hooks modify the replaced text behind our back, how can the Lisp
program which does that expect to have point where it belongs?  Am I
missing something?



Reply | Threaded
Open this post in threaded view
|

bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement

Lars Ingebrigtsen
Eli Zaretskii <[hidden email]> writes:

>> Uhm...  is that comment wrong?  Aren't we moving point to the end of the
>> inserted replacement?
>
> Yes, it should say "end", not "start".

Fixed now.

> Personally, I wonder what was expected here.  If the modification
> hooks modify the replaced text behind our back, how can the Lisp
> program which does that expect to have point where it belongs?  Am I
> missing something?

No, the semantics are pretty unclear, and it's not obvious whether we
can guarantee anything here.  But the bug reporter notes:

> Other types of modification (insert or delete) do not have this issue.
> `point' is adjusted before running modification hooks.

So I think the suggestion is to move point to the end of the replacement
before running the modification hooks, i.e., move the point logic to
replace_range.  But I'm not actually sure where the modification hooks
are being run from...

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no