Re: emacs-27 3bce7ec: CC Mode: Protect against consecutive calls to before-change-functions ...

Re: emacs-27 3bce7ec: CC Mode: Protect against consecutive calls to before-change-functions ...

[Glenn Morris]

Alan Mackenzie wrote:

> branch: emacs-27
> commit 3bce7ec3826003fda1971224a20d7fe2cba8bf65
>
>     CC Mode: Protect against consecutive calls to before-change-functions ...

This causes srecode-utest-getset-output in
test/lisp/cedet/srecode-utest-getset.el to fail:

Test srecode-utest-getset-output backtrace:
  parse-partial-sexp(1 627 nil nil nil syntax-table)
  c-after-change-mark-abnormal-strings(100 320 0)
  #f(compiled-function (fn) #<bytecode 0x9b5689>)(c-after-change-mark-
  mapc(#f(compiled-function (fn) #<bytecode 0x9b5689>) (c-depropertize
  c-after-change(100 320 0)
  run-hook-with-args(c-after-change 100 320 0)
  srecode-insert-fcn(#<srecode-template getset-in-class> #<srecode-dic
  srecode-insert-getset()
  #f(compiled-function () #<bytecode 0x9573ad>)()
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name srecode-utest-getset-output :document
  ert-run-or-rerun-test(#s(ert--stats :selector (not (or ... ...)) :te
  ert-run-tests((not (or (tag :expensive-test) (tag :unstable))) #f(co
  ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable)))
  ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
  eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
  command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/cedet/srecode-utest-
  command-line()
  normal-top-level()
Test srecode-utest-getset-output condition:
    (args-out-of-range #<buffer srecode-utest-getset-TkO46e.cpp> 1 627)
   FAILED  1/1  srecode-utest-getset-output (1.709802 sec)

Re: emacs-27 3bce7ec: CC Mode: Protect against consecutive calls to before-change-functions ...

[Mattias Engdegård]

28 feb. 2020 kl. 17.54 skrev Glenn Morris <rgm@gnu.org>:

>>   CC Mode: Protect against consecutive calls to before-change-functions ...
> 
> This causes srecode-utest-getset-output in
> test/lisp/cedet/srecode-utest-getset.el to fail:

Indeed, and so does electric-tests. Condensed reproduction:

(with-temp-buffer
  (c-mode)
  (insert "a")
  (comment-region (point-min) (point-max)))

Alan, it looks like the code has lost control over c-new-END. c-after-change over-adjusts it to a value beyond the buffer size.
The problem seems to go away with the expedient below but it is unlikely to be the right solution.

--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -2006,7 +2006,8 @@ c-after-change
 
   ;; (c-new-BEG c-new-END) will be the region to fontify.  It may become
   ;; larger than (beg end).
-  (setq c-new-END (- (+ c-new-END (- end beg)) old-len))
+  (setq c-new-END (min (- (+ c-new-END (- end beg)) old-len)
+		       (point-max)))
 
   (unless (c-called-from-text-property-change-p)
     (setq c-just-done-before-change nil)

Re: emacs-27 3bce7ec: CC Mode: Protect against consecutive calls to before-change-functions ...

[Alan Mackenzie]

Hello, Mattias and Glenn.

On Sun, Mar 01, 2020 at 12:49:39 +0100, Mattias Engdegård wrote:
> 28 feb. 2020 kl. 17.54 skrev Glenn Morris <rgm@gnu.org>:

> >>   CC Mode: Protect against consecutive calls to before-change-functions ...

> > This causes srecode-utest-getset-output in
> > test/lisp/cedet/srecode-utest-getset.el to fail:

> Indeed, and so does electric-tests. Condensed reproduction:

> (with-temp-buffer
>   (c-mode)
>   (insert "a")
>   (comment-region (point-min) (point-max)))

> Alan, it looks like the code has lost control over c-new-END.
> c-after-change over-adjusts it to a value beyond the buffer size.  The
> problem seems to go away with the expedient below but it is unlikely
> to be the right solution.

The cause of this bug is a bug in combine-change-calls-1 which shows
itself when buffer-undo-list is t (as I presume it is for the
with-temp-buffer).  In this circumstance, the routine fails to bind
before/after-change-functions to nil (or something close to it) before
invoking combine-change-call's ,@body.

So, we get two invocations of before-change-functions in a row, which
fouls things up.

I have tried a fix to combine-change-calls-1, and it appears to work.  I
will tidy it up and post it here soon (?tomorrow).

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: emacs-27 3bce7ec: CC Mode: Protect against consecutive calls to before-change-functions ...

[Alan Mackenzie]

Hello, Mattias.

On Sun, Mar 01, 2020 at 12:49:39 +0100, Mattias Engdegård wrote:
> 28 feb. 2020 kl. 17.54 skrev Glenn Morris <rgm@gnu.org>:

> >>   CC Mode: Protect against consecutive calls to
> >>   before-change-functions ...

> > This causes srecode-utest-getset-output in
> > test/lisp/cedet/srecode-utest-getset.el to fail:

> Indeed, and so does electric-tests. Condensed reproduction:

> (with-temp-buffer
>   (c-mode)
>   (insert "a")
>   (comment-region (point-min) (point-max)))

Actually, your scenario and Glenn's were caused by different bugs.  ;-)

> Alan, it looks like the code has lost control over c-new-END.

Yes.

> c-after-change over-adjusts it to a value beyond the buffer size.  The
> problem seems to go away with the expedient below but it is unlikely
> to be the right solution.

It wasn't.  ;-)  I've fixed both bugs in the emacs-27 branch.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: emacs-27 3bce7ec: CC Mode: Protect against consecutive calls to before-change-functions ...

[Alan Mackenzie]

Hello, Glenn.

On Fri, Feb 28, 2020 at 11:54:23 -0500, Glenn Morris wrote:
> Alan Mackenzie wrote:

> > branch: emacs-27
> > commit 3bce7ec3826003fda1971224a20d7fe2cba8bf65

> >     CC Mode: Protect against consecutive calls to before-change-functions ...

> This causes srecode-utest-getset-output in
> test/lisp/cedet/srecode-utest-getset.el to fail:

> Test srecode-utest-getset-output backtrace:
>   parse-partial-sexp(1 627 nil nil nil syntax-table)
>   c-after-change-mark-abnormal-strings(100 320 0)
>   #f(compiled-function (fn) #<bytecode 0x9b5689>)(c-after-change-mark-
>   mapc(#f(compiled-function (fn) #<bytecode 0x9b5689>) (c-depropertize
>   c-after-change(100 320 0)
>   run-hook-with-args(c-after-change 100 320 0)
>   srecode-insert-fcn(#<srecode-template getset-in-class> #<srecode-dic
>   srecode-insert-getset()
>   #f(compiled-function () #<bytecode 0x9573ad>)()
>   ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>   ert-run-test(#s(ert-test :name srecode-utest-getset-output :document
>   ert-run-or-rerun-test(#s(ert--stats :selector (not (or ... ...)) :te
>   ert-run-tests((not (or (tag :expensive-test) (tag :unstable))) #f(co
>   ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable)))
>   ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
>   eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
>   command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/cedet/srecode-utest-
>   command-line()
>   normal-top-level()
> Test srecode-utest-getset-output condition:
>     (args-out-of-range #<buffer srecode-utest-getset-TkO46e.cpp> 1 627)
>    FAILED  1/1  srecode-utest-getset-output (1.709802 sec)

Yes.  I've fixed this bug in the emacs-27 branch.

As a matter of interest, the scenario Mattias reported was caused by a
different bug (also fixed).

-- 
Alan Mackenzie (Nuremberg, Germany).