bug#11749: Acknowledgement (24.1; C-mode indentation gives wrong-type-argument error.)

[Alan Mackenzie]

Hi, Michael,

On Wed, Oct 10, 2012 at 08:00:25PM +0000, Alan Mackenzie wrote:
> On Tue, Oct 09, 2012 at 10:05:07AM -0400, Michael Welsh Duggan wrote:
> > Alan Mackenzie <acm@muc.de> writes:

[ .... ]

> > > I have found the bug which is causing (most of) these dings, though I
> > > don't think it is the one which caused Kim's original bug.  Could you try
> > > out the patch below, please.  (I have also enhanced/corrected the
> > > debugging routines a bit, too.)

> > Still doesn't seem to help much here.  I have attached a file which
> > reliably causes a cache failure.  I have attached the smallest file of
> > the set of files I am working on that causes this particular problem.
> > Load the attached file and toggle on parse state debugging.  Then scroll
> > to the bottom of the file.  (Use C-v multiple times, or just M->.)  One
> > reason I have attached this file is that it only triggers the warning
> > message once.  Most of my larger files cause this to happen quite a lot.

> What is happening in this file is another bug, arising from historical
> assumptions which are no longer valid.

> The "from scratch" calculation notes that the starting scanning position
> would be a long way (>5000) back, hence it tries going back to the second
> "beginning-of-defun" to get a top-level starting point.  This
> "beginning-of-defun" is a pure "brace in column zero" test.

> This doesn't work in C++ when constructs inside a namespace have braces
> at column zero, something I believe has become very common in recent
> years.  Namespaces didn't exist in C++ when c-parse-state was originally
> written.

> Obviously this optimisation is no longer valid.  I wouldn't be surprised
> if it has caused quite a bit of buggy behaviour.  I'll need to think it
> over for a few days to decide what to do.

The only reasonable thing to do is to disable the heuristic for C++ Mode.
This is what the patch below now does.  Could you try it out as usual,
please.  Thanks!




diff -r ac6584d14c06 cc-engine.el
--- a/cc-engine.el      Sun Sep 09 11:15:13 2012 +0000
+++ b/cc-engine.el      Sun Oct 14 17:02:08 2012 +0000
@@ -2560,8 +2560,11 @@
              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
@@ -2648,17 +2651,19 @@
        ;; 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))
+                    (> 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
@@ -3354,13 +3359,19 @@
   (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)
 (make-variable-buffer-local 'c-parse-state-state)
 (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
@@ -3373,7 +3384,8 @@
           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 "
@@ -3416,7 +3428,8 @@
       (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")


> > -- 
> > Michael Welsh Duggan
> > (mwd@cert.org)

-- 
Alan Mackenzie (Nuremberg, Germany).