Discussion:
bug#16368: 24.3; freeze in cperl mode when editing a regexp
(too old to reply)
Stefan Kangas
2019-09-20 23:33:08 UTC
Permalink
/(\d{4})(?: +){2}/;
e.g.: "emacs -Q file" then M-x cperl-mode <RET>
Put the cursor over the second opening brace, and type <DEL>
several times to delete what is before "{2}". When one types
/(\d{4})(?:{2}/;
^cursor
Emacs freezes. One can type C-g to interrupt.
I can confirm this is still an issue in Emacs 26.1 and on the current
master branch.

Best regards,
Stefan Kangas
Harald Jörg
2020-09-02 20:06:40 UTC
Permalink
That bug exists since 14 years, but only surfaces if the regexp has a
closing brace in the wrong place (two characters before an unfinished
group starts). The reason is that `cperl-forward-group-in-re' failed
to report unbalanced parentheses: In the function, the error was too
narrowly localized and gets lost before returning.

Patch attached, including two test cases.
--
Cheers,
haj
Stefan Kangas
2020-09-02 22:19:25 UTC
Permalink
Post by Harald Jörg
Patch attached, including two test cases.
Thanks for working on cperl-mode bugs.

Testing the original recipe with your patch it is already an improvement
in that it avoids the freeze. But I see these messages:

cperl-forward-group-in-re: error (scan-error Unbalanced parentheses 9 94)
(scan-error Unbalanced parentheses 9 94)

Is that the expected and desired behavior?

---
Post by Harald Jörg
=?UTF-8?q?steo.de>?=
Better first line:

Fix freeze in cperl-mode when editing a regexp
Post by Harald Jörg
* lisp/progmodes/cperl-mode.el (cperl-forward-group-in-re): Make
sure that an error is reported back to the caller (Bug#16368).
Tests for balanced (no error) and unbalanced (caught exception)
cases of `cperl-forward-group-in-re'.
Format these flush to the first column (no initial space).
Post by Harald Jörg
--- a/test/lisp/progmodes/cperl-mode-tests.el
+++ b/test/lisp/progmodes/cperl-mode-tests.el
I see this:

ELC lisp/progmodes/cperl-mode-tests.elc

In end of data:
lisp/progmodes/cperl-mode-tests.el:73:1: Warning: the function
`cperl-forward-group-in-re' is not known to be defined.

You should add this line to the top:

(require 'cperl-mode)

Best regards,
Stefan Kangas
Harald Jörg
2020-09-02 23:40:45 UTC
Permalink
Post by Stefan Kangas
Post by Harald Jörg
Patch attached, including two test cases.
Thanks for working on cperl-mode bugs.
Testing the original recipe with your patch it is already an improvement
cperl-forward-group-in-re: error (scan-error Unbalanced parentheses 9 94)
(scan-error Unbalanced parentheses 9 94)
Is that the expected and desired behavior?
Yes, it is, sort of. Maybe it should be fine-tuned. At this point we
_have_ the situation of unbalanced parentheses, and cperl-mode rubs it
in for every character you type.

Just open a buffer in cperl-mode and start typing:

$a =~ s/

At this point a message appears, with a different text when you open
a parentheses, and will haunt you until you get everything closed
properly. Given that regexps can be messy and heavy with punctuation,
I'd say this is desired behavior.

I can only guess that the first part of the message (which starts with
"cperl-forward-group-in-re") was added out of frustration: The bug
prevented the second part of the message (without
"cperl-forward-group-in-re") from ever appearing. Only this wasn't
fatal unless... there was this closing brace two characters before.
I'll check that, and prepare an updated patch if that's true.
Post by Stefan Kangas
---
Post by Harald Jörg
=?UTF-8?q?steo.de>?=
Fix freeze in cperl-mode when editing a regexp
Post by Harald Jörg
* lisp/progmodes/cperl-mode.el (cperl-forward-group-in-re): Make
sure that an error is reported back to the caller (Bug#16368).
Tests for balanced (no error) and unbalanced (caught exception)
cases of `cperl-forward-group-in-re'.
Format these flush to the first column (no initial space).
No problem, I'll do so. I thought I was supposed to create the commit
messages with C-x 4 a, but probably I misunderstood and should have
post-processed that text in the first place.
Post by Stefan Kangas
Post by Harald Jörg
--- a/test/lisp/progmodes/cperl-mode-tests.el
+++ b/test/lisp/progmodes/cperl-mode-tests.el
ELC lisp/progmodes/cperl-mode-tests.elc
lisp/progmodes/cperl-mode-tests.el:73:1: Warning: the function
`cperl-forward-group-in-re' is not known to be defined.
(require 'cperl-mode)
Hm. That should rather be _moving_ that line to the top? The line is
there, in the test which calls this function. For me this seemed to
be enough to avoid that message when byte-compiling. But of course,
moving the line to the top is fine, probably more tests will follow
to exercise functions which aren't autoloaded.

Give me a few hours for a nap: It's past midnight here :)
--
Cheers,
haj
Stefan Kangas
2020-09-03 09:58:40 UTC
Permalink
Post by Harald Jörg
I'd say this is desired behavior.
OK, thanks. Your explanation sounds good to me.
Post by Harald Jörg
I can only guess that the first part of the message (which starts with
"cperl-forward-group-in-re") was added out of frustration: The bug
prevented the second part of the message (without
"cperl-forward-group-in-re") from ever appearing. Only this wasn't
fatal unless... there was this closing brace two characters before.
I'll check that, and prepare an updated patch if that's true.
Sounds good.
Post by Harald Jörg
No problem, I'll do so. I thought I was supposed to create the commit
messages with C-x 4 a, but probably I misunderstood and should have
post-processed that text in the first place.
I always use C-x 4 a, and then delete the spacing to the left.
Post by Harald Jörg
Hm. That should rather be _moving_ that line to the top?
Ah, right. Yup, that sounds good.

Loading...