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

[Alan Mackenzie]

Hello again, Eric.

On Sun, Nov 10, 2019 at 09:47:35 -0800, Eric Scrivner wrote:
> I've applied the patch and run emacs with it. Unfortunately, it doesn't
> seem to solve the issue. One the bright side, I believe I had buffers which
> can reliably reproduce the issue (at least locally). I've copied one such
> file below. The issue can be reproduced by simply going to the end of
> `main` and attempting to indent the `SQL_Quit()` statement. At first it
> attempts to overindent the line as `statement-cont`, then if I go up and
> attempt to indent the `PlatformLog` in the else statement then return to
> the SDL_Quit and indent it, it has become `topmost-intro`. Apologies for
> the long length of the file, but hopefully it can help with reproduction:

I've had a look at the bug and there was a non-sensical function in CC
Mode fouling things up.  In the patch below, I've rewritten this
function.  Could I ask you please to apply the new patch (to the
original Emacs 26.3 version off cc-engine.el), byte-compile it and test
it.  Then please let me know how it goes.  However ....

I would strongly advise you to set the Emacs variable
open-parens-in-column-0-is-defun-start to nil.  Whether for C++ Mode in
general (in a hook function), for this one file in particular (in a
Local Variables: section at the end of the file), or globally.

With open-p......-start left at its default of t, Emacs will search
backward for the beginning of a function by searching for an open
paren/brace/bracket at column 0.  It deems the found paren/brace/bracket
the start of a function even if that p/b/b is inside a string or
function.  This seems likely to happen in your test file, where you've
got such braces at column 0 inside two raw strings.

This variable codifies a difficult dilemma for Emacs.  Without the paren
in column 0 heuristic, the beginning of function search would have to
scan to the beginning of the buffer every time which is somewhat slow
(perhaps less slow now on modern PCs, but still slow).  With the
heuristic, analysis problems happen when there are braces at col 0 which
aren't BO functions (e.g., in C++ Mode when things inside a namespace
start at column 0, a popular convention).  For a time, o-p-i-c-0-i-d-s
was spiked to nil by CC Mode, but this led to complaints of slowness.

Anyhow, here's the improved patch.  I look forward to hearing back from
you.  Thanks for all the time you've spent on this bug.



diff -r 2783baa48d44 cc-engine.el
--- a/cc-engine.el      Fri Oct 25 20:00:14 2019 +0000
+++ b/cc-engine.el      Wed Nov 13 18:13:11 2019 +0000
@@ -3371,19 +3371,35 @@
   (or (car (c-state-literal-at pos))
       pos))
 
-(defsubst c-state-cache-non-literal-place (pos state)
-  ;; Return a position outside of a string/comment/macro at or before POS.
-  ;; STATE is the parse-partial-sexp state at POS.
-  (let ((res (if (or (nth 3 state)     ; in a string?
-                    (and (nth 4 state)
-                         (not (eq (nth 7 state) 'syntax-table)))) ; in a 
comment?
-                (nth 8 state)
-              pos)))
+(defun c-state-cache-lower-good-pos (here pos state)
+  ;; Return a good pos (in the sense of `c-state-cache-good-pos') at the
+  ;; lowest[*] position between POS and HERE which is syntactically equivalent
+  ;; to HERE.  This position may be HERE itself.  POS is before HERE in the
+  ;; buffer.
+  ;; [*] We don't actually always determine this exact position, since this
+  ;; would require a disproportionate amount of work, given that this function
+  ;; deals only with a corner condition, and POS and HERE are typically on
+  ;; adjacent lines.  We actually return either POS, when POS is a good
+  ;; position, HERE otherwise.  Exceptionally, when POS is in a comment, but
+  ;; HERE not, we can return the position of the end of the comment.
+  (let (s)
     (save-excursion
-      (goto-char res)
-      (if (c-beginning-of-macro)
-         (point)
-       res))))
+      (goto-char pos)
+      (when (nth 8 state)      ; POS in a comment or string.  Move out of it.
+       (setq s (parse-partial-sexp pos here nil nil state 'syntax-table))
+       (when (< (point) here)
+         (setq pos (point)
+               state s)))
+      (if (eq (point) here)            ; HERE is in the same literal as POS
+         pos
+       (setq s (parse-partial-sexp pos here (1+ (car state)) nil state nil))
+       (cond
+        ((> (car s) (car state))  ; Moved into a paren between POS and HERE
+         here)
+        ((not (eq (nth 6 s) (car state))) ; Moved out of a paren between POS
+                                       ; and HERE
+         here)
+        (t pos))))))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; Stuff to do with point-min, and coping with any literal there.
@@ -3690,7 +3706,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 +3815,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 +4021,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 +4158,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 {.
@@ -4385,7 +4408,7 @@
       (setq c-state-cache-good-pos
            (if (and bopl-state
                     (< good-pos (- here c-state-cache-too-far)))
-               (c-state-cache-non-literal-place here-bopl bopl-state)
+               (c-state-cache-lower-good-pos here here-bopl bopl-state)
              good-pos)))
 
      ((eq strategy 'backward)


-- 
Alan Mackenzie (Nuremberg, Germany).