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

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Basil L. Contovounesios
reassign 45375 emacs,cc-mode
quit

[CCed CC Mode maintainer.]

[hidden email], Géza <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

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

> [hidden email], Géza <[hidden email]> 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).



Reply | Threaded
Open this post in threaded view
|

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

Herman
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



Reply | Threaded
Open this post in threaded view
|

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



Reply | Threaded
Open this post in threaded view
|

bug#45375: 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).