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

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

[Herman, Géza]

On current master (6af31fd71ff1a403c199c479577bcc145a547db1) indentation 
of C/C++ files sometimes doesn't work. I've bisected it: commit 
"9022df7027 Optimise c-parse-state for large buffers with few (if any) 
braces." introduced this behavior.

This is how to reproduce: check out 
9022df70270243f211c54ccd66800320148b8434, and execute "emacs -Q 
xdisp.c". Jump to line 2989 with M-g M-g 2989, move the cursor to the 
end of line of "Lisp_Object retval;", and press enter. The cursor will 
be moved to the correct place (correctly indented, cursor will be placed 
below the 'L' character of the previous line). Then push enter at end of 
line of "va_list ap;". For me, cursor will jump to the beginning of the 
line, it won't be indented. If I keep pressing enters, the next failure 
will be at "va_end (ap);". I'm not sure whether this exact steps 
reproduces for everyone, but it happened me 5 of 5 trials. If I don't 
press enter at the first line ("Lisp_Object retval;"), the problem 
doesn't happen for any of this function lines. But it will happen for 
somewhere else, if I keep trying (move around the file, press enter at 
random places: if will fail sooner or later).

For my configuration (without -Q), this problem happens quite frequently 
during editing C++ code.

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

[Basil L. Contovounesios]

reassign 45375 emacs,cc-mode
quit

[CCed CC Mode maintainer.]

Herman@debbugs.gnu.org, Géza <geza.herman@gmail.com> writes:

> On current master (6af31fd71ff1a403c199c479577bcc145a547db1) indentation of
> C/C++ files sometimes doesn't work. I've bisected it: commit "9022df7027
> Optimise c-parse-state for large buffers with few (if any) braces." introduced
> this behavior.
>
> This is how to reproduce: check out 9022df70270243f211c54ccd66800320148b8434,
> and execute "emacs -Q xdisp.c". Jump to line 2989 with M-g M-g 2989, move the
> cursor to the end of line of "Lisp_Object retval;", and press enter. The cursor
> will be moved to the correct place (correctly indented, cursor will be placed
> below the 'L' character of the previous line). Then push enter at end of line of
> "va_list ap;". For me, cursor will jump to the beginning of the line, it won't
> be indented. If I keep pressing enters, the next failure will be at "va_end
> (ap);". I'm not sure whether this exact steps reproduces for everyone, but it
> happened me 5 of 5 trials. If I don't press enter at the first line
> ("Lisp_Object retval;"), the problem doesn't happen for any of this function
> lines. But it will happen for somewhere else, if I keep trying (move around the
> file, press enter at random places: if will fail sooner or later).
>
> For my configuration (without -Q), this problem happens quite frequently during
> editing C++ code.

I tried following your recipe (the lines in xdisp.c have since moved
around a bit), but was unable to reproduce the issue.

Can you still reproduce it on your end?

Thanks,

-- 
Basil

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

[Alan Mackenzie]

Hello, Basil, Géza and Konstantin.

Sorry I missed this bug report in December, and thanks, Basil, for
drawing it to my attention.

On Fri, Feb 12, 2021 at 21:17:56 +0000, Basil L. Contovounesios wrote:
> reassign 45375 emacs,cc-mode
> quit

> [CCed CC Mode maintainer.]

> Herman@debbugs.gnu.org, Géza <geza.herman@gmail.com> writes:

> > On current master (6af31fd71ff1a403c199c479577bcc145a547db1) indentation of
> > C/C++ files sometimes doesn't work. I've bisected it: commit "9022df7027
> > Optimise c-parse-state for large buffers with few (if any) braces." introduced
> > this behavior.

> > This is how to reproduce: check out 9022df70270243f211c54ccd66800320148b8434,
> > and execute "emacs -Q xdisp.c". Jump to line 2989 with M-g M-g 2989, move the
> > cursor to the end of line of "Lisp_Object retval;", and press enter. The cursor
> > will be moved to the correct place (correctly indented, cursor will be placed
> > below the 'L' character of the previous line). Then push enter at end of line of
> > "va_list ap;". For me, cursor will jump to the beginning of the line, it won't
> > be indented. If I keep pressing enters, the next failure will be at "va_end
> > (ap);". I'm not sure whether this exact steps reproduces for everyone, but it
> > happened me 5 of 5 trials. If I don't press enter at the first line
> > ("Lisp_Object retval;"), the problem doesn't happen for any of this function
> > lines. But it will happen for somewhere else, if I keep trying (move around the
> > file, press enter at random places: if will fail sooner or later).

> > For my configuration (without -Q), this problem happens quite frequently during
> > editing C++ code.

> I tried following your recipe (the lines in xdisp.c have since moved
> around a bit), but was unable to reproduce the issue.

> Can you still reproduce it on your end?

I can reproduce it easily on the indicated commit, and I have found
where the problem is.  It's in the handling of the c-state-cache (the
cache which tracks the positions of certain
braces/brackets/parentheses).  Fixing it could be quite tricky,
particularly given the need to retain optimisation for the case that the
above commit "fixed".

I am fairly, but not absolutely, sure that this is the same bug as
46400, but the bug scenario here is easier to reproduce than the other
one, so I will concentrate on this bug first.  Maybe we can join the bug
reports later.

> Thanks,

> -- 
> Basil

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Herman, Géza]

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.

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

Thank you for looking at this!
Geza

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).

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

[Alan Mackenzie]

Hello, Géza.

On Sun, Feb 14, 2021 at 11:11:28 +0000, Alan Mackenzie wrote:
> On Sat, Feb 13, 2021 at 19:43:31 +0100, Herman, Géza wrote:

[ .... ]

> 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.

I have now applied the patch to all the relevant places, and I am
closing the bugs with this post.

[ .... ]

> > Geza

-- 
Alan Mackenzie (Nuremberg, Germany).