Re: Unbalanced change hooks (part 2)

[Alan Mackenzie <acm@muc.de>]

Hello, Eli.

On Mon, Aug 08, 2016 at 08:17:42PM +0300, Eli Zaretskii wrote:
> > Date: Mon, 8 Aug 2016 16:54:49 +0000
> > Cc: ofv@wanadoo.es, rcopley@gmail.com, rms@gnu.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > What triggers that code?

> > before/after-change-functions.  For example, in C++ Mode, if a "<"
> > template delimiter is about is in a region about to be deleted, and the
> > matching ">" is after that region, the pertinent function called from
> > c-before-change (c-before-change-check-<>-operators) removes the
> > syntax-table properties from each of them.

> > > E.g., when a file is visited, what invokes that code, and how much of
> > > the buffer it processes when invoked?

> > On a newly visited file, the entire buffer is scanned by (the components
> > of) c-before-change and c-after-change, setting the text properties.

> So, if worse comes to worst, you could trigger such a complete scan
> after revert-buffer, e.g. in a specialized revert-buffer-function.

Why would that be bad?  We detect the bad condition by two consecutive
after-c-f calls, check that revert-buffer is on the call stack, and
twiddle `beg' `end' `old-len' (the parameters to after-change functions)
to indicate the entire buffer.  Then let c-after-change carry on.  This
feels too easy.  :-)  What am I missing?

> Do you have a way of completely forgetting everything about a certain
> region of buffer text, and rescanning that region anew?

This is tricky, because, for example, template delimiters outside that
region are associated with deleted (or unmarked) delimiters inside the
region.

> If so, would it work to do this in c-after-change, since its arguments
> tell you both the old and the new buffer positions where the changes
> were made?  If this works, it might be less expensive then rescanning
> the entire buffer.

I don't think this would work.  beg, end, and old-len only say where
things were changed, not what syntax-table'd characters have been
deleted.  Anyway, what's so bad about scanning the entire buffer after
having reverted it?

> > What I'm looking at at the moment is calling revert-buffer a second time
> > if CC Mode signals a missing before-change-functions the first time.
> > After the test scenario, M-x revert-buffer works, so it might work if
> > called from the code instead.

> I don't understand how this could work reliably: it has the same
> problems as the original recipe.  Am I missing something?

There is nothing like buffer-undo-list which can be corrupted by other
programs.  Anyhow, I've implemented it, and it works well on the test
case from bug #24094.  Here is the embryonic patch (I haven't reindented
code, so as to keep the patch short, and there's message output).  It
works, for the moment, by putting "around" advice around revert-buffer,
and invoking ad-do-it a second time if the first time didn't work:




diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 04d2ed6..4db71b6 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -492,10 +492,13 @@ c-maybe-stale-found-type
 (defvar c-just-done-before-change nil)
 (make-variable-buffer-local 'c-just-done-before-change)
 ;; This variable is set to t by `c-before-change' and to nil by
-;; `c-after-change'.  It is used to detect a spurious invocation of
-;; `before-change-functions' directly following on from a correct one.  This
-;; happens in some Emacsen, for example when `basic-save-buffer' does (insert
-;; ?\n) when `require-final-newline' is non-nil.
+;; `c-after-change'.  It is used for two purposes: (i) to detect a spurious
+;; invocation of `before-change-functions' directly following on from a
+;; correct one.  This happens in some Emacsen, for example when
+;; `basic-save-buffer' does (insert ?\n) when `require-final-newline' is
+;; non-nil; (ii) to detect when Emacs fails to invoke
+;; `before-change-functions'.  This can happend when reverting a buffer - see
+;; bug #24094.
 
 (defun c-basic-common-init (mode default-style)
   "Do the necessary initialization for the syntax handling routines
@@ -643,8 +646,9 @@ c-basic-common-init
   (add-hook 'after-change-functions 'c-after-change nil t)
   (when (boundp 'font-lock-extend-after-change-region-function)
     (set (make-local-variable 'font-lock-extend-after-change-region-function)
-         'c-extend-after-change-region))) ; Currently (2009-05) used by all
+         'c-extend-after-change-region)) ; Currently (2009-05) used by all
                           ; languages with #define (C, C++,; ObjC), and by AWK.
+  (add-hook 'pre-command-hook 'c-pre-command-hook t))
 
 (defun c-setup-doc-comment-style ()
   "Initialize the variables that depend on the value of `c-doc-comment-style'."
@@ -1225,6 +1229,22 @@ c-before-change
     ;; newly inserted parens would foul up the invalidation algorithm.
     (c-invalidate-state-cache beg)))
 
+(defvar c-missed-before-change nil)
+
+(defun c-pre-command-hook ()
+  (setq c-missed-before-change nil))
+
+(defadvice revert-buffer (around c-advise-revert-buffer activate)
+  ad-do-it
+  (when (and c-buffer-is-cc-mode
+	     c-missed-before-change)
+    (message "Repeating revert-buffer.")
+    (setq c-missed-before-change nil)
+    ad-do-it
+    (message "Repeated revert-buffer %s"
+	     (if c-missed-before-change "failed" "succeeded"))
+    ))
+
 (defvar c-in-after-change-fontification nil)
 (make-variable-buffer-local 'c-in-after-change-fontification)
 ;; A flag to prevent region expanding stuff being done twice for after-change
@@ -1244,6 +1264,10 @@ c-after-change
   ;; This calls the language variable c-before-font-lock-functions, if non nil.
   ;; This typically sets `syntax-table' properties.
 
+  ;; Rarely, Emacs will fail to call `before-change-functions'.  See bug
+  ;; #24094.  In such cases, do it "by hand".
+  (if (not c-just-done-before-change)
+      (setq c-missed-before-change t)
   ;; (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))
@@ -1291,7 +1315,7 @@ c-after-change
 	  (save-excursion
 	    (mapc (lambda (fn)
 		    (funcall fn beg end old-len))
-		  c-before-font-lock-functions)))))))
+		  c-before-font-lock-functions))))))))
 
 (defun c-fl-decl-start (pos)
   ;; If the beginning of the line containing POS is in the middle of a "local"
@@ -1483,6 +1507,8 @@ c-extend-after-change-region
   ;; Of the seven CC Mode languages, currently (2009-05) only C, C++, Objc
   ;; (the languages with #define) and AWK Mode make non-null use of this
   ;; function.
+  (if c-missed-before-change
+      (cons beg end)
   (when (eq font-lock-support-mode 'jit-lock-mode)
     (save-restriction
       (widen)
@@ -1491,7 +1517,7 @@ c-extend-after-change-region
 	    (put-text-property c-new-BEG beg 'fontified nil))
 	(if (> c-new-END end)
 	    (put-text-property end c-new-END 'fontified nil)))))
-  (cons c-new-BEG c-new-END))
+  (cons c-new-BEG c-new-END)))
 
 ;; Emacs < 22 and XEmacs
 (defmacro c-advise-fl-for-region (function)


-- 
Alan Mackenzie (Nuremberg, Germany).