bug#37910: CC Mode 5.33.2 (C++//l); CC-mode inconsistently indents everything as topmost-intro after a while

[Alan Mackenzie <acm@muc.de>]

Hello, Eric.

Now for a top-post.  ;-)

I think I may have found the cause of the bug.  It may have been caused
by overwriting lisp list structure, rather than creating new (parallel)
list structures - a sort of corruption.  See below for a fuller
explanation.  This hypothesis is entirely consistent with the observed
result (spurious topmost-intro's).

Would you please apply the patch below and byte-compile the result.  It
suffices just to compile cc-engine.el.  (If you want any help with
applying the patch or byte compiling, feel free to send me private
email.)

Then please try out the patched CC Mode for however long it has taken,
in the past, to see the sort of bug you reported, and then somewhat
longer.  If the bug fails to show itself, we may well have fixed it.
Please let me know how things are going.

############## Optional section.  An explanation #######################

One of the caches CC Mode uses, "c-state-cache", keeps track of the
nested brace structure.  It is (re)calculated on calling the lisp
function c-parse-state.  In essence, it records the position of each
enclosing brace in a list, the most nested first.  So if we had the
following C++ structure:

    { // 1
    ......
        { // 2
	......
	    { // 3
	    ...... <point>

, and <point> was at the marked position, our c-state-cache would be the
list of the three brace positions (P3 P2 P1).  One of its uses is
determining if some point is at the top level or not.  If the
c-state-cache list for that point is empty, it is at the top level.

As point moves through the buffer, and c-parse-state is called from
somewhere else, c-state-cache is "altered" in a highly optimised
fashion.  This avoids having to scan large portions of the buffer too
often, to determine the brace structure.  The nature of this "altering"
is what is causing the problem.

When the buffer is narrowed, say beginning with the brace 2, calling
c-parse-state now has to return (P3 P2), because the brace 1 is now
outside the visible portion.

The suspected bug cause is the way (P3 P2 P1) is changed to (P3 P2) on
this narrowing.  Up to now the list structure itself has been changed,
rather than making a copy of the structure.

So, what may have been happening is that CC Mode is looping through the
c-state-cache to determine whether point is at top level.  (Being
directly inside a class or namespace, etc., counts as "top level").  If
point is inside brace 1, the loop will try to determine that P1 is not a
class, etc., and return "not at top level".  However, if c-parse-state
had been called with the above narrowing, c-state-cache is now (P3 P2),
point appears to be outside every brace, and the loop spuriously returns
"at top level".  This is what I think has been happening.

When the code wrongly reports "at top level", we get the unwanted
topmost-intro analyses.

The solution to this is when the buffer is narrowed and we call
c-parse-state, we make a COPY of the c-state-cache list, leaving the
original unmolested for its original owner.  This is what the patch
does.

############## End of optional section. ################################

Here is the patch.  It should work in Emacs-26.3, even though the line
numbers are now a bit different:



diff -r 2783baa48d44 cc-engine.el
--- a/cc-engine.el	Fri Oct 25 20:00:14 2019 +0000
+++ b/cc-engine.el	Sun Nov 10 10:30:17 2019 +0000
@@ -3690,7 +3690,13 @@
 							; brace pair.
 	    (setq c-state-cache nil
 		  c-state-cache-good-pos c-state-min-scan-pos)
-	  (setcdr ptr nil)
+	  ;; Do not alter the original `c-state-cache' structure, since there
+	  ;; may be a loop suspended which is looping through that structure.
+	  ;; This may have been the cause of bug #37910.
+	  (let ((cdr-ptr (cdr ptr)))
+	    (setcdr ptr nil)
+	    (setq c-state-cache (copy-sequence c-state-cache))
+	    (setcdr ptr cdr-ptr))
 	  (setq c-state-cache-good-pos (1+ (c-state-cache-top-lparen))))
 	)))
 
@@ -3793,11 +3799,12 @@
 		(setq new-cons (cons bra (1+ ce)))
 		(cond
 		 ((consp (car c-state-cache))
-		  (setcar c-state-cache new-cons))
+		  (setq c-state-cache (cons new-cons (cdr c-state-cache))))
 		 ((and (numberp (car c-state-cache)) ; probably never happens
 		       (< ce (car c-state-cache)))
-		  (setcdr c-state-cache
-			  (cons new-cons (cdr c-state-cache))))
+		  (setq c-state-cache
+			(cons (car c-state-cache)
+			      (cons new-cons (cdr c-state-cache)))))
 		 (t (setq c-state-cache (cons new-cons c-state-cache)))))
 
 	    ;; We haven't found a brace pair.  Record this in the cache.
@@ -3998,7 +4005,7 @@
 	(when (and c-state-cache
 		   (consp (car c-state-cache))
 		   (> (cdar c-state-cache) upper-lim))
-	  (setcar c-state-cache (caar c-state-cache))
+	  (setq c-state-cache (cons (caar c-state-cache) (cdr c-state-cache)))
 	  (setq scan-back-pos (car c-state-cache)
 		cons-separated t))
 
@@ -4135,7 +4142,7 @@
       ;; knowledge of what's inside these braces, we have no alternative but
       ;; to direct the caller to scan the buffer from the opening brace.
       (setq pos (caar c-state-cache))
-      (setcar c-state-cache pos)
+      (setq c-state-cache (cons pos (cdr c-state-cache)))
       (list (1+ pos) pos t)) ; return value.  We've just converted a brace pair
 			     ; entry into a { entry, so the caller needs to
 			     ; search for a brace pair before the {.




On Sun, Oct 27, 2019 at 15:39:56 +0000, Alan Mackenzie wrote:
> Hello, Eric.

> On Thu, Oct 24, 2019 at 12:06:18 -0700, Eric Scrivner wrote:
> > This seems related to (if not the same as) bug #5490.

> > - This happens randomly and then randomly stops happening (cache expiry
> > maybe?)
> > - M-x revert-buffer does not fix.

> Thanks for taking the trouble to report this bug, and thanks even more
> for including so much information.

> > Here's a snippet from the buffer at the time this happen, as you can see
> > there seems to be a region until the end where everything becomes
> > topmost-intro:

> >       }                          // ((block-close 18328))
> >                                 // ((statement 9560))
> >       SDL_DestroyWindow(Window); // ((statement 9560))
> >     }    // ((block-close 9490))
> >     else                        // ((else-clause 9466))
> >     {                                                             //
> > ((substatement-open 18464))
> >       PlatformLog("Failed to create window: %s", SDL_GetError()); //
> > ((statement-block-intro 18473))
> >     } // ((block-close 18473))
> >                                 // ((topmost-intro 18576))
> >                                 // ((topmost-intro 18576))
> > SDL_Quit();                     // ((topmost-intro 18541))
> >   }                             // ((topmost-intro 18548))
> >   else                          // ((else-clause 9188))
> >   {                             // ((substatement-open 18845))
> >     PlatformLog("Failed to initialize SDL: %s", SDL_GetError()); //
> > ((statement-block-intro 18901))((statement-block-intro 18724))
> >   }                                                              //
> > ((block-close 18901))
> >                                 // ((topmost-intro 19093))
> >   return EXIT_SUCCESS;          // ((topmost-intro 19093))
> > }                               // ((topmost-intro 19242))

> At the moment, all I can do is acknowledge receipt of your report.  It's
> obviously not an easy bug to diagnose.

> I can see two ways of making progress: (i) Inspecting
> c-guess-basic-syntax, the function which analyses code and produces
> (amongs other things) all these topmost-intro's.  It is essentially a
> large cond form (Lisp's equivalent of a switch), and the single place
> which produces topmost-intro comes fairly early on in this cond form;
> (ii) Determine what aspects of a buffer do not get reinitialised after
> evaluating M-x revert-buffer.  That could provide some clue.

> [ CC Mode config dump acknowledged with thanks, but snipped ]

> Just one thing.  If you haven't already done so, could you make a backup
> copy of a buffer which triggers the bug, just in case after some future
> edit it no longer does so.  Thanks!

-- 
Alan Mackenzie (Nuremberg, Germany).