GNU bug report logs - #23127
25.0.92; Assertion failure when editing history in query-replace in cc-mode

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 23127 in the body.
You can then email your comments to 23127 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: Richard Copley <rcopley <at> gmail.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: 25.0.92;
 Assertion failure when editing history in query-replace in cc-mode
Date: Sun, 27 Mar 2016 20:51:42 +0100

In cc-mode, while doing query-replace, edit a previous replacement in
the minibuffer, and insert characters after the " → " separator. The
subsequent emacs-lisp chokes on the text properties of the resulting
string, with an assertion failure.

Recipe starting from 'emacs -Q':

M-x c-mode RET
M-<
C-% a RET b RET !
M-<
C-% M-p C-e <backspace> c RET

==> error "cl--assertion-failed: Assertion failed: (not
(text-property-any (1+ split-pos) length (quote separator) t string))"

The callstack looks like this (except that here I've replaced a NUL with "^@"):

Debugger entered: ((cl-assertion-failed (not (text-property-any (1+
split-pos) length (quote separator) t string)) nil))
  cl--assertion-failed((not (text-property-any (1+ split-pos) length
(quote separator) t string)))
  query-replace--split-string(#("a^@c" 1 2 (display #(" → " 0 3 (face
minibuffer-prompt)) separator t) 2 3 (separator t)))
  query-replace-read-from("Query replace" nil)
  query-replace-read-args("Query replace" nil)
  (let ((common (query-replace-read-args (concat "Query replace" (if
current-prefix-arg (if (eq current-prefix-arg ...) " backward" "
word") "") (if (use-region-p) " in region" "")) nil))) (list (nth 0
common) (nth 1 common) (nth 2 common) (if (use-region-p)
(region-beginning)) (if (use-region-p) (region-end)) (nth 3 common)
(if (use-region-p) (region-noncontiguous-p))))
  call-interactively(query-replace nil nil)
  command-execute(query-replace)

As you can see, the "c" inherited the separator property, and
query-replace--split-string choked on that.
It doesn't happen in all modes. It does happen in c-mode and c++-mode.

In GNU Emacs 25.0.92.1 (x86_64-w64-mingw32)
 of 2016-03-21 built on MACHINE
Repository revision: 76ef52267cf887e3e1aa6d25b3b16dd0601dd459
Windowing system distributor 'Microsoft Corp.', version 10.0.10586
Configured using:
 'configure --prefix /c/emacs/emacs-20160321-102640
 --without-imagemagick --disable-dependency-tracking
 --enable-locallisppath=%emacs_dir%/../site-lisp 'CFLAGS=-Og -g -ggdb''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND DBUS NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS

Important settings:
  value of $LANG: ENG
  locale-coding-system: cp1252

Major mode: C++/l

Minor modes in effect:
  tooltip-mode: t
  global-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
  abbrev-mode: t

Recent messages:
Mark set
cl--assertion-failed: Assertion failed: (not (text-property-any (1+
split-pos) length (quote separator) t string))

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns mail-prsvr mail-utils
thingatpt cl-extra help-mode cc-mode cc-fonts easymenu cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs cl-loaddefs pcase
cl-lib kmacro time-date mule-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp disp-table
w32-win w32-vars term/common-win tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core 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 charscript case-table epa-hook
jka-cmpr-hook help simple abbrev 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 w32notify dbusbind w32 multi-tty
make-network-process emacs)

Memory information:
((conses 16 110212 13535)
 (symbols 56 21829 0)
 (miscs 48 49 129)
 (strings 32 21738 3341)
 (string-bytes 1 713085)
 (vectors 16 15026)
 (vector-slots 8 455559 3738)
 (floats 8 164 97)
 (intervals 56 275 18)
 (buffers 976 12))

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

From: Alan Mackenzie <acm <at> muc.de>
To: Richard Copley <rcopley <at> gmail.com>
Cc: 23127 <at> debbugs.gnu.org, acm <at> muc.de
Subject: Re: bug#23127: 25.0.92;
 Assertion failure when editing history in query-replace in cc-mode
Date: 28 Mar 2016 22:02:10 -0000

Hello, Richard.

In article <mailman.8469.1459108386.843.bug-gnu-emacs <at> gnu.org> you wrote:
> In cc-mode, while doing query-replace, edit a previous replacement in
> the minibuffer, and insert characters after the " → " separator. The
> subsequent emacs-lisp chokes on the text properties of the resulting
> string, with an assertion failure.

> Recipe starting from 'emacs -Q':

> M-x c-mode RET
> M-<
> C-% a RET b RET !
> M-<
> C-% M-p C-e <backspace> c RET

> ==> error "cl--assertion-failed: Assertion failed: (not
> (text-property-any (1+ split-pos) length (quote separator) t string))"

> The callstack looks like this (except that here I've replaced a NUL with "^@"):

> Debugger entered: ((cl-assertion-failed (not (text-property-any (1+
> split-pos) length (quote separator) t string)) nil))
>   cl--assertion-failed((not (text-property-any (1+ split-pos) length
> (quote separator) t string)))
>   query-replace--split-string(#("a^@c" 1 2 (display #(" → " 0 3 (face
> minibuffer-prompt)) separator t) 2 3 (separator t)))
>   query-replace-read-from("Query replace" nil)
>   query-replace-read-args("Query replace" nil)
>   (let ((common (query-replace-read-args (concat "Query replace" (if
> current-prefix-arg (if (eq current-prefix-arg ...) " backward" "
> word") "") (if (use-region-p) " in region" "")) nil))) (list (nth 0
> common) (nth 1 common) (nth 2 common) (if (use-region-p)
> (region-beginning)) (if (use-region-p) (region-end)) (nth 3 common)
> (if (use-region-p) (region-noncontiguous-p))))
>   call-interactively(query-replace nil nil)
>   command-execute(query-replace)

> As you can see, the "c" inherited the separator property, and
> query-replace--split-string choked on that.
> It doesn't happen in all modes. It does happen in c-mode and c++-mode.

Thanks for such a high quality bug report.  The cause was a critical
variable being buffer local in C Mode and C++ Mode.  This buffer local
binding was not available in `read-from-minibuffer', with the result that
the `separator' text property got spread to the newly inserted 'c'.

The following patch should fix the bug.  Would you please try it out and
confirm that the bug is indeed fixed, or report what's still wrong.
Thanks!

> In GNU Emacs 25.0.92.1 (x86_64-w64-mingw32)
>  of 2016-03-21 built on MACHINE
> Repository revision: 76ef52267cf887e3e1aa6d25b3b16dd0601dd459
> Windowing system distributor 'Microsoft Corp.', version 10.0.10586
> Configured using:
>  'configure --prefix /c/emacs/emacs-20160321-102640
>  --without-imagemagick --disable-dependency-tracking
>  --enable-locallisppath=%emacs_dir%/../site-lisp 'CFLAGS=-Og -g -ggdb''



diff --git a/lisp/replace.el b/lisp/replace.el
index 428be3c..412f827 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -167,8 +167,6 @@ query-replace-read-from
     ;; unavailable while preparing to dump.
     (custom-reevaluate-setting 'query-replace-from-to-separator)
     (let* ((history-add-new-input nil)
-	   (text-property-default-nonsticky
-	    (cons '(separator . t) text-property-default-nonsticky))
 	   (separator
 	    (when query-replace-from-to-separator
 	      (propertize "\0"
@@ -195,9 +193,16 @@ query-replace-read-from
 	    (save-excursion
 	      (if regexp-flag
 		  (read-regexp prompt nil 'query-replace-from-to-history)
-		(read-from-minibuffer
-		 prompt nil nil nil 'query-replace-from-to-history
-		 (car (if regexp-flag regexp-search-ring search-ring)) t))))
+                ;; The `with-current-buffer' ensures that the binding
+                ;; for `text-property-default-nonsticky' isn't a
+                ;; buffer local binding in the current buffer, which
+                ;; `read-from-minibuffer' wouldn't see.
+                (with-current-buffer (window-buffer (minibuffer-window))
+                  (let ((text-property-default-nonsticky
+                         (cons '(separator . t) text-property-default-nonsticky)))
+                    (read-from-minibuffer
+                     prompt nil nil nil 'query-replace-from-to-history
+                     (car (if regexp-flag regexp-search-ring search-ring)) t))))))
            (to))
       (if (and (zerop (length from)) query-replace-defaults)
 	  (cons (caar query-replace-defaults)



-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Richard Copley <rcopley <at> gmail.com>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 23127 <at> debbugs.gnu.org
Subject: Re: bug#23127: 25.0.92; Assertion failure when editing history in
 query-replace in cc-mode
Date: Tue, 29 Mar 2016 00:24:20 +0100

On 28 March 2016 at 23:02, Alan Mackenzie <acm <at> muc.de> wrote:
> Hello, Richard.
>
> In article <mailman.8469.1459108386.843.bug-gnu-emacs <at> gnu.org> you wrote:
>> In cc-mode, while doing query-replace, edit a previous replacement in
>> the minibuffer, and insert characters after the " → " separator. The
>> subsequent emacs-lisp chokes on the text properties of the resulting
>> string, with an assertion failure.
>
>> Recipe starting from 'emacs -Q':
>
>> M-x c-mode RET
>> M-<
>> C-% a RET b RET !
>> M-<
>> C-% M-p C-e <backspace> c RET
>
>> ==> error "cl--assertion-failed: Assertion failed: (not
>> (text-property-any (1+ split-pos) length (quote separator) t string))"
>
>> The callstack looks like this (except that here I've replaced a NUL with "^@"):
>
>> Debugger entered: ((cl-assertion-failed (not (text-property-any (1+
>> split-pos) length (quote separator) t string)) nil))
>>   cl--assertion-failed((not (text-property-any (1+ split-pos) length
>> (quote separator) t string)))
>>   query-replace--split-string(#("a^@c" 1 2 (display #(" → " 0 3 (face
>> minibuffer-prompt)) separator t) 2 3 (separator t)))
>>   query-replace-read-from("Query replace" nil)
>>   query-replace-read-args("Query replace" nil)
>>   (let ((common (query-replace-read-args (concat "Query replace" (if
>> current-prefix-arg (if (eq current-prefix-arg ...) " backward" "
>> word") "") (if (use-region-p) " in region" "")) nil))) (list (nth 0
>> common) (nth 1 common) (nth 2 common) (if (use-region-p)
>> (region-beginning)) (if (use-region-p) (region-end)) (nth 3 common)
>> (if (use-region-p) (region-noncontiguous-p))))
>>   call-interactively(query-replace nil nil)
>>   command-execute(query-replace)
>
>> As you can see, the "c" inherited the separator property, and
>> query-replace--split-string choked on that.
>> It doesn't happen in all modes. It does happen in c-mode and c++-mode.
>
> Thanks for such a high quality bug report.  The cause was a critical
> variable being buffer local in C Mode and C++ Mode.  This buffer local
> binding was not available in `read-from-minibuffer', with the result that
> the `separator' text property got spread to the newly inserted 'c'.
>
> The following patch should fix the bug.  Would you please try it out and
> confirm that the bug is indeed fixed, or report what's still wrong.
> Thanks!
>
>> In GNU Emacs 25.0.92.1 (x86_64-w64-mingw32)
>>  of 2016-03-21 built on MACHINE
>> Repository revision: 76ef52267cf887e3e1aa6d25b3b16dd0601dd459
>> Windowing system distributor 'Microsoft Corp.', version 10.0.10586
>> Configured using:
>>  'configure --prefix /c/emacs/emacs-20160321-102640
>>  --without-imagemagick --disable-dependency-tracking
>>  --enable-locallisppath=%emacs_dir%/../site-lisp 'CFLAGS=-Og -g -ggdb''
>
>
>
> diff --git a/lisp/replace.el b/lisp/replace.el
> index 428be3c..412f827 100644
> --- a/lisp/replace.el
> +++ b/lisp/replace.el
> @@ -167,8 +167,6 @@ query-replace-read-from
>      ;; unavailable while preparing to dump.
>      (custom-reevaluate-setting 'query-replace-from-to-separator)
>      (let* ((history-add-new-input nil)
> -          (text-property-default-nonsticky
> -           (cons '(separator . t) text-property-default-nonsticky))
>            (separator
>             (when query-replace-from-to-separator
>               (propertize "\0"
> @@ -195,9 +193,16 @@ query-replace-read-from
>             (save-excursion
>               (if regexp-flag
>                   (read-regexp prompt nil 'query-replace-from-to-history)
> -               (read-from-minibuffer
> -                prompt nil nil nil 'query-replace-from-to-history
> -                (car (if regexp-flag regexp-search-ring search-ring)) t))))
> +                ;; The `with-current-buffer' ensures that the binding
> +                ;; for `text-property-default-nonsticky' isn't a
> +                ;; buffer local binding in the current buffer, which
> +                ;; `read-from-minibuffer' wouldn't see.
> +                (with-current-buffer (window-buffer (minibuffer-window))
> +                  (let ((text-property-default-nonsticky
> +                         (cons '(separator . t) text-property-default-nonsticky)))
> +                    (read-from-minibuffer
> +                     prompt nil nil nil 'query-replace-from-to-history
> +                     (car (if regexp-flag regexp-search-ring search-ring)) t))))))
>             (to))
>        (if (and (zerop (length from)) query-replace-defaults)
>           (cons (caar query-replace-defaults)

Thanks very much Alan.
I tested your patch and noticed that it fixes query-replace but not
query-replace-regexp. I won't submit a new patch because I don't have
the paperwork (sorry), but I reckon you just need to move the
"(with-current-buffer (...) (let (...) ...))" construct outside the
"(if regexp-flag ...)". With that change it works fine for me.

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

From: Alan Mackenzie <acm <at> muc.de>
To: Richard Copley <rcopley <at> gmail.com>
Cc: 23127 <at> debbugs.gnu.org
Subject: Re: bug#23127: 25.0.92; Assertion failure when editing history in
 query-replace in cc-mode
Date: Tue, 29 Mar 2016 10:03:09 +0000

Hello, Richard.

On Tue, Mar 29, 2016 at 12:24:20AM +0100, Richard Copley wrote:

> Thanks very much Alan.
> I tested your patch and noticed that it fixes query-replace but not
> query-replace-regexp.

Whoops!

> I won't submit a new patch because I don't have the paperwork (sorry),
> but I reckon you just need to move the "(with-current-buffer (...)
> (let (...) ...))" construct outside the "(if regexp-flag ...)". With
> that change it works fine for me.

And it works fine for me, too.  Thanks for spotting that.  I've
committed the fix (to the emacs-25 branch in the savannah repository),
and I'm closing the bug.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Alan Mackenzie <acm <at> muc.de>
To: 23127-done <at> debbugs.gnu.org
Subject: Re: bug#23127: 25.0.92; Assertion failure when editing history in
 query-replace in cc-mode
Date: Tue, 29 Mar 2016 10:04:27 +0000

Bug fixed.

-- 
Alan Mackenzie (Nuremberg, Germany).

Previous Next

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