Fix bugs in the CC Mode state cache. Enhance a debugging mechanism.

3 files changed, 65 insertions, 12 deletions

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 8502cd477e9..e60e9b49043 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,20 @@
+2012-11-21  Alan Mackenzie  <acm@muc.de>
+
+        Fix bugs in the state cache.  Enhance a debugging mechanism.
+        * progmodes/cc-engine.el (c-parse-state-get-strategy): Don't use
+        "brace at column zero" strategy for C++.
+        (c-append-lower-brace-pair-to-state-cache): Repair algorithm.
+        (c-parse-state-point): New variable.
+        (c-record-parse-state-state): Record old parse state with
+        `copy-tree'.  Record previous value of point.
+        (c-debug-parse-state-double-cons): New debugging function.
+        (c-debug-parse-state): Call the above new function.
+        (c-toggle-parse-state-debug): Output a confirmatory message.
+
+        * progmodes/cc-mode.el (c-before-change, c-after-change): Call
+        c-invalidate-state-cache from `c-before-change' instead of
+        `c-after-change'.
+
 2012-11-20  Daniel Colascione  <dancol@dancol.org>
 
         * term/w32-win.el (cygwin-convert-path-from-windows): Accomodate
diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index 2aa04cb2b0b..10355451480 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -2561,8 +2561,11 @@ comment at the start of cc-engine.el for more info."
               start-point cache-pos)))
 
     ;; Might we be better off starting from the top level, two defuns back,
-    ;; instead?
-    (when (> how-far c-state-cache-too-far)
+    ;; instead?  This heuristic no longer works well in C++, where
+    ;; declarations inside namespace brace blocks are frequently placed at
+    ;; column zero.
+    (when (and (not (c-major-mode-is 'c++-mode))
+               (> how-far c-state-cache-too-far))
       (setq BOD-pos (c-get-fallback-scan-pos here)) ; somewhat EXPENSIVE!!!
       (if (< (- here BOD-pos) how-far)
           (setq strategy 'BOD
@@ -2649,17 +2652,20 @@ comment at the start of cc-engine.el for more info."
         ;; If we're essentially repeating a fruitless search, just give up.
         (unless (and c-state-brace-pair-desert
                      (eq cache-pos (car c-state-brace-pair-desert))
+                     (or (null (car c-state-brace-pair-desert))
+                         (> from (car c-state-brace-pair-desert)))
                      (<= from (cdr c-state-brace-pair-desert)))
-          ;; DESERT-LIM.  Only search what we absolutely need to,
+          ;; DESERT-LIM.  Avoid repeated searching through the cached desert.
           (let ((desert-lim
                  (and c-state-brace-pair-desert
                       (eq cache-pos (car c-state-brace-pair-desert))
+                      (>= from (cdr c-state-brace-pair-desert))
                       (cdr c-state-brace-pair-desert)))
                 ;; CACHE-LIM.  This limit will be necessary when an opening
                 ;; paren at `cache-pos' has just had its matching close paren
-                ;; inserted.  `cache-pos' continues to be a search bound, even
-                ;; though the algorithm below would skip over the new paren
-                ;; pair.
+                ;; inserted into the buffer.  `cache-pos' continues to be a
+                ;; search bound, even though the algorithm below would skip
+                ;; over the new paren pair.
                 (cache-lim (and cache-pos (< cache-pos from) cache-pos)))
             (narrow-to-region
                 (cond
@@ -3342,12 +3348,18 @@ comment at the start of cc-engine.el for more info."
   (fset 'c-real-parse-state (symbol-function 'c-parse-state)))
 (cc-bytecomp-defun c-real-parse-state)
 
+(defvar c-parse-state-point nil)
 (defvar c-parse-state-state nil)
 (defun c-record-parse-state-state ()
+  (setq c-parse-state-point (point))
   (setq c-parse-state-state
         (mapcar
          (lambda (arg)
-           (cons arg (symbol-value arg)))
+           (let ((val (symbol-value arg)))
+             (cons arg
+                   (if (consp val)
+                       (copy-tree val)
+                     val))))
          '(c-state-cache
            c-state-cache-good-pos
            c-state-nonlit-pos-cache
@@ -3360,7 +3372,8 @@ comment at the start of cc-engine.el for more info."
            c-state-point-min-lit-start
            c-state-min-scan-pos
            c-state-old-cpp-beg
-           c-state-old-cpp-end))))
+           c-state-old-cpp-end
+           c-parse-state-point))))
 (defun c-replay-parse-state-state ()
   (message
    (concat "(setq "
@@ -3370,6 +3383,16 @@ comment at the start of cc-engine.el for more info."
      c-parse-state-state "  ")
     ")")))
 
+(defun c-debug-parse-state-double-cons (state)
+  (let (state-car conses-not-ok)
+    (while state
+      (setq state-car (car state)
+            state (cdr state))
+      (if (and (consp state-car)
+               (consp (car state)))
+          (setq conses-not-ok t)))
+    conses-not-ok))
+
 (defun c-debug-parse-state ()
   (let ((here (point)) (res1 (c-real-parse-state)) res2)
     (let ((c-state-cache nil)
@@ -3402,8 +3425,16 @@ comment at the start of cc-engine.el for more info."
                here res1 res2)
       (message "Old state:")
       (c-replay-parse-state-state))
+
+    (when (c-debug-parse-state-double-cons res1)
+      (message "c-parse-state INVALIDITY at %s: %s"
+               here res1)
+      (message "Old state:")
+      (c-replay-parse-state-state))
+
     (c-record-parse-state-state)
-    res1))
+    res2 ; res1 correct a cascading series of errors ASAP
+    ))
 
 (defun c-toggle-parse-state-debug (&optional arg)
   (interactive "P")
@@ -3411,7 +3442,9 @@ comment at the start of cc-engine.el for more info."
   (fset 'c-parse-state (symbol-function (if c-debug-parse-state
                                             'c-debug-parse-state
                                           'c-real-parse-state)))
-  (c-keep-region-active))
+  (c-keep-region-active)
+  (message "c-debug-parse-state %sabled"
+           (if c-debug-parse-state "en" "dis")))
 (when c-debug-parse-state
   (c-toggle-parse-state-debug 1))
 
diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 50eaebe4dec..91866278e28 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -1034,7 +1034,10 @@ Note that the style variables are always made local to the buffer."
             (mapc (lambda (fn)
                     (funcall fn beg end))
                   c-get-state-before-change-functions))
-        ))))
+        )))
+  ;; The following must be done here rather than in `c-after-change' because
+  ;; newly inserted parens would foul up the invalidation algorithm.
+  (c-invalidate-state-cache beg))
 
 (defvar c-in-after-change-fontification nil)
 (make-variable-buffer-local 'c-in-after-change-fontification)
@@ -1082,7 +1085,7 @@ Note that the style variables are always made local to the buffer."
 
         (c-trim-found-types beg end old-len) ; maybe we don't need all of these.
         (c-invalidate-sws-region-after beg end)
-        (c-invalidate-state-cache beg)
+        ;; (c-invalidate-state-cache beg) ; moved to `c-before-change'.
         (c-invalidate-find-decl-cache beg)
 
         (when c-recognize-<>-arglists