bug#45973: replace-regexp lisp replacement bug

bug#45973: replace-regexp lisp replacement bug

[Nicholas Drozd]

Here is a string representing a Turing machine program:

  1RB 1LC 1RC 1RB 1RD 0LE 1LA 1LD 1RH 0LA

Say I have a file full of lines like this, and I want to swap all the
Ls for Rs and vice versa. `replace-regexp` is a good way to do it. For
the regexp, I use `\(\(L\)\|\(R\)\)`, a matching group with two nested
matching alternatives; for the replacement, I use a Lisp expression:
`\,(if \2 "R" "L")`. This expression says: if the second match is
found (the `L`), replace it with "R", else replace it with "L".

In Emacs 27 (b58fd1eab9), this gives the expected result:

  1LB 1RC 1LC 1LB 1LD 0RE 1RA 1RD 1LH 0RA

But in Emacs 28 (20add1cd22), it gives an error:

  replace-highlight: Wrong type argument: integer-or-marker-p, nil

A simpler replacement does work: `\(L\)\|R` and `\,(if \1 "R" "L")`.
But the longer one should work too.

I tried to make a Lisp test to reproduce this, but the `\,`
replacement feature is interactive-only, and I don't know how to test
it non-interactively.

bug#45973: replace-regexp lisp replacement bug

[Lars Ingebrigtsen]

Nicholas Drozd <nicholasdrozd@gmail.com> writes:

> I tried to make a Lisp test to reproduce this, but the `\,`
> replacement feature is interactive-only, and I don't know how to test
> it non-interactively.

The reproducer is:

(replace-regexp "\\(\\(L\\)\\|\\(R\\)\\)" '(replace-eval-replacement replace-quote (if (match-string 2) "R" "L")) nil nil nil nil nil)

And this leads to the following backtrace:

Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
  make-overlay(nil nil)
  replace-highlight(2 3 nil nil "\\(\\(L\\)\\|\\(R\\)\\)" t nil nil nil)
  perform-replace("\\(\\(L\\)\\|\\(R\\)\\)" (replace-eval-replacement replace-quote (if (match-string 2) "R" "L")) nil t nil nil nil nil nil nil nil)
  replace-regexp("\\(\\(L\\)\\|\\(R\\)\\)" (replace-eval-replacement replace-quote (if (match-string 2) "R" "L")) nil nil nil nil nil)
  eval((replace-regexp "\\(\\(L\\)\\|\\(R\\)\\)" '(replace-eval-replacement replace-quote (if (match-string 2) "R" "L")) nil nil nil nil nil) t)
  eval-expression((replace-regexp "\\(\\(L\\)\\|\\(R\\)\\)" '(replace-eval-replacement replace-quote (if (match-string 2) "R" "L")) nil nil nil nil nil) nil nil 127)
  funcall-interactively(eval-expression (replace-regexp "\\(\\(L\\)\\|\\(R\\)\\)" '(replace-eval-replacement replace-quote (if (match-string 2) "R" "L")) nil nil nil nil nil) nil nil 127)
  call-interactively(eval-expression nil nil)
  command-execute(eval-expression)

So this is a problem with the new code that highlights the matches, I
think?

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

bug#45973: replace-regexp lisp replacement bug

[Juri Linkov]

tags 45973 fixed
close 45973 28.0.50
thanks

> Nicholas Drozd <nicholasdrozd@gmail.com> writes:
>> I tried to make a Lisp test to reproduce this, but the `\,`
>> replacement feature is interactive-only, and I don't know how to test
>> it non-interactively.

Thanks for the reproducible test case.

> The reproducer is:
>
> (replace-regexp "\\(\\(L\\)\\|\\(R\\)\\)" '(replace-eval-replacement replace-quote (if (match-string 2) "R" "L")) nil nil nil nil nil)
>
> So this is a problem with the new code that highlights the matches, I
> think?

Yep.  Now fixed on master.

bug#45973: replace-regexp lisp replacement bug

[Nicholas Drozd]

[-- Attachment #1: Type: text/plain, Size: 79 bytes --]

Thanks for the quick turnaround. I've included a patch to add a test for this.

[-- Attachment #2: 0001-test-lisp-replace-tests.el-Add-nested-match-group-te.patch --]
[-- Type: text/x-patch, Size: 1209 bytes --]

From d7bed718a5a88cc834456ac8d2e39ffc4f07e292 Mon Sep 17 00:00:00 2001
From: Nick Drozd <nicholasdrozd@gmail.com>
Date: Tue, 19 Jan 2021 16:26:02 -0600
Subject: [PATCH] * test/lisp/replace-tests.el: Add nested match group test

---
 test/lisp/replace-tests.el | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/test/lisp/replace-tests.el b/test/lisp/replace-tests.el
index 8c2682a1f1..2db570c97d 100644
--- a/test/lisp/replace-tests.el
+++ b/test/lisp/replace-tests.el
@@ -587,5 +587,18 @@ occur-highlight-occurrence
                               (get-text-property (point) 'occur-target))
           (should (funcall check-overlays has-overlay)))))))
 
+(ert-deftest replace-regexp-bug45973 ()
+  "Test for https://debbugs.gnu.org/45973 ."
+  (let ((before "1RB 1LC 1RC 1RB 1RD 0LE 1LA 1LD 1RH 0LA")
+        (after  "1LB 1RC 1LC 1LB 1LD 0RE 1RA 1RD 1LH 0RA"))
+    (with-temp-buffer
+      (insert before)
+      (goto-char (point-min))
+      (replace-regexp
+       "\\(\\(L\\)\\|\\(R\\)\\)"
+       '(replace-eval-replacement
+         replace-quote
+         (if (match-string 2) "R" "L")))
+      (should (equal (buffer-string) after)))))
 
 ;;; replace-tests.el ends here
-- 
2.17.1

bug#45973: replace-regexp lisp replacement bug

[Lars Ingebrigtsen]

Nicholas Drozd <nicholasdrozd@gmail.com> writes:

> Thanks for the quick turnaround. I've included a patch to add a test for this.

Thanks; added to Emacs 28.

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