bug#45375: cc-mode indentation sometimes doesn't work

[Alan Mackenzie]

Hello, Géza.

On Sat, Feb 13, 2021 at 19:43:31 +0100, Herman, Géza wrote:
> Hi all,

> >> Can you still reproduce it on your end?

> I'm using a ~3-week-old native-comp branch, this bug (#45375) doesn't 
> happen anymore for me.

The bug was caused by an error in the handling of a cache (the
"c-state-cache" which tracks the position of certain
braces/brackets/parenthese).  I can assure you the error is still there,
even if it isn't currently triggering very often.

> Note that the bisect gave a different commit for #46400, so maybe the 
> issues are different, even though the symptoms are very similar (same?).

In bug #46400, I think Konstantin's bisecting threw up the wrong commit,
since cache bugs tend to be very slippery to pin down.

> Thank you for looking at this!

A pleasure!  I intend to commit the patch below in a few days time, if I
don't hear back from anybody that it's giving trouble.  This patch fixes
the bug when applied to that commit from December
(9022df70270243f211c54ccd66800320148b8434).  It should apply cleanly to
master.

> Geza


Hello, Konstantin.

>> I don't think the bug was introduced by the commit you cite, more
>> likely that commit triggered the bug which was lying in wait elsewhere.

>> I've been working on this bug for several hours, so far, and have found
>> that the "c-state-cache" (which records the positions of certain
>> braces, brackets and parentheses) becomes corrupt while running your
>> `test' function.  I'm trying to track down where and how this
>> corruption happens.

>> Also relevant is that the bug seems to be being triggered by the
>> apostrophe in

>>     bar("Couldn't open %s", to);
>>                ^

>> ..  At least, if I take that apostrophe away, I don't see the bug
>> symptoms any more.

Actually, I was mistaken on this front - the apostrophe had nothing to do
with it.

>> So, please bear with me some while longer.  I am working on the bug.

> D: Oh, this is cool! Sure, thank you very much for looking into it! I'm
> definitely not in hurry, I was just kinda being afraid that the report
> could've gotten through the cracks. I'm happy to hear that wasn't the
> case �

It was indeed a bug in the c-state-cache handling, and I now have a patch
to fix it.  After applying the patch, the `test' function no longer
produces a newline without indentation.  There is something complicated
going on with `undo', so that `test' sometimes reports there is no more
undo data, but the indentation left on new lines is always correct.

As I said above, I intend to commit the patch below in a few days time.
Please feel free to apply it (to master) and test it out.  I would be
happy to hear of anything to do with this bug which is still not working.




diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index 68dadcc272..653c4332b6 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -4314,38 +4314,29 @@ c-invalidate-state-cache-1
       (setq c-state-nonlit-pos-cache-limit (1- here)))
   (c-truncate-lit-pos-cache here)
 
-  ;; `c-state-cache':
-  ;; Case 1: if `here' is in a literal containing point-min, everything
-  ;; becomes (or is already) nil.
-  (if (or (null c-state-cache-good-pos)
-         (< here (c-state-get-min-scan-pos)))
-      (setq c-state-cache nil
-           c-state-cache-good-pos nil
-           c-state-min-scan-pos nil)
-
-    ;; Truncate `c-state-cache' and set `c-state-cache-good-pos' to a value
-    ;; below `here'.  To maintain its consistency, we may need to insert a new
-    ;; brace pair.
-    (let ((here-bol (c-point 'bol here))
-         too-high-pa  ; recorded {/(/[ next above or just below here, or nil.
-         dropped-cons)           ; was the last removed element a brace pair?
-      ;; The easy bit - knock over-the-top bits off `c-state-cache'.
-      (while (and c-state-cache
-                 (>= (c-state-cache-top-paren) here))
-       (setq dropped-cons (consp (car c-state-cache))
-             too-high-pa (c-state-cache-top-lparen)
-             c-state-cache (cdr c-state-cache)))
-
-      ;; Do we need to add in an earlier brace pair, having lopped one off?
-      (if (and dropped-cons
-              (<= too-high-pa here))
-         (c-append-lower-brace-pair-to-state-cache too-high-pa here here-bol))
-      (if (and c-state-cache-good-pos (< here c-state-cache-good-pos))
-         (setq c-state-cache-good-pos
-               (or (save-excursion
-                     (goto-char here)
-                     (c-literal-start))
-                   here)))))
+  (cond
+   ;; `c-state-cache':
+   ;; Case 1: if `here' is in a literal containing point-min, everything
+   ;; becomes (or is already) nil.
+   ((or (null c-state-cache-good-pos)
+       (< here (c-state-get-min-scan-pos)))
+    (setq c-state-cache nil
+         c-state-cache-good-pos nil
+         c-state-min-scan-pos nil))
+
+   ;; Case 2: `here' is below `c-state-cache-good-pos', so we need to amend
+   ;; the entire `c-state-cache' data.
+   ((< here c-state-cache-good-pos)
+    (let* ((res (c-remove-stale-state-cache-backwards here))
+          (good-pos (car res))
+          (scan-backward-pos (cadr res))
+          (scan-forward-p (car (cddr res))))
+      (if scan-backward-pos
+         (c-append-lower-brace-pair-to-state-cache scan-backward-pos here))
+      (setq c-state-cache-good-pos
+           (if scan-forward-p
+               (c-append-to-state-cache good-pos here)
+             good-pos)))))
 
   ;; The brace-pair desert marker:
   (when (car c-state-brace-pair-desert)


-- 
Alan Mackenzie (Nuremberg, Germany).