Subject: Re: bug#11749: Acknowledgement (24.1; C-mode indentation gives wrong-type-argument error.)

From: Alan Mackenzie <[email-masked]>
Date: Wed, 23 Jan 2013 14:16:45 +0000

Hi, Michael.

On Thu, Jan 17, 2013 at 11:27:04AM -0500, Michael Welsh Duggan wrote:
> Alan Mackenzie <[email-masked]> writes:

> [This] fixes this case, indeed.  Unfortunately, here's another.

> emacs -Q rwtransfer.c
>   M-x c-toggle-parse-state-debug
>   M->
> Then type M-v until the parse failure happens.  For me it happens at 
> 32% point in the file.  Please not that this particular problem only
> happens when paging through the file in reverse order.

> c-parse-state inconsistency at 15885: using cache: (15636 15181 (13849 . 15141)), from scratch: (15636 (15303 . 15493) 15181 (13849 . 15141))
> Old state:
> (setq c-state-cache '(16334 16271 16248 (16059 . 16193) 15181 (13849 . 15141))  c-state-cache-good-pos 16335  c-state-nonlit-pos-cache '(48473 45473 42473 39473 36473 33473 30473 27378 24378 21260 18015 15015 12015 9015 6015 3015)  c-state-nonlit-pos-cache-limit 48473  c-state-semi-nonlit-pos-cache '(48323 45323 42323 39323 36323 33260 30260 27260 24260 21260 18001 15001 12001 9001 6001 3001)  c-state-semi-nonlit-pos-cache-limit 48323  c-state-brace-pair-desert '(15181 . 17388)  c-state-point-min 1  c-state-point-min-lit-type nil  c-state-point-min-lit-start nil  c-state-min-scan-pos 1  c-state-old-cpp-beg nil  c-state-old-cpp-end nil  c-parse-state-point 16335)
> c-parse-state inconsistency at 15885: using cache: (15636 15181 (13849 . 15141)), from scratch: (15636 (15303 . 15493) 15181 (13849 . 15141))

I think the following patch should fix that glitch.  I've also taken the
opportunity to simplify things a little (part of the global state was
being passed needlessly as a parameter) and to rename some variables for
more consistency.

Would you try this out and confirm it works, as usual, please.

However, this isn't the end of the story - 

(i) Start your test case in rwtransfer.c, and M-v until the top of the
  file.  (This shouldn't display any inconsistency messages)
(ii) M-x goto-char <ret> 20001 <ret>.  This should go to EOL 671, just
  after a "}".
(iii) Type <space> }.  (Two characters)
(iv) C-v, possibly twice.

This displays a message something like:
    c-parse-state inconsistency at 21070: using cache: ((20458 . 20935)),
    from scratch: ((20838 . 20877))
, together with a state dump.

This bug isn't a new one, but I've just come across it.  I'll be working
on it in the meantime.

Here's the patch, based on the savannah emacs-24 branch (which should be
identical to the trunk):

=== modified file 'lisp/progmodes/cc-engine.el'
*** lisp/progmodes/cc-engine.el	2013-01-09 21:33:00 +0000
--- lisp/progmodes/cc-engine.el	2013-01-23 13:36:33 +0000
***************
*** 2545,2558 ****
    ;;
    ;; The return value is a list, one of the following:
    ;;
!   ;; o - ('forward CACHE-POS START-POINT) - scan forward from START-POINT,
!   ;;                                        which is not less than CACHE-POS.
!   ;; o - ('backward CACHE-POS nil) - scan backwards (from HERE).
!   ;; o - ('BOD nil START-POINT) - scan forwards from START-POINT, which is at the
    ;;   top level.
!   ;; o - ('IN-LIT nil nil) - point is inside the literal containing point-min.
!   ;; , where CACHE-POS is the highest position recorded in `c-state-cache' at
!   ;; or below HERE.
    (let ((cache-pos (c-get-cache-scan-pos here))	; highest position below HERE in cache (or 1)
  	BOD-pos		    ; position of 2nd BOD before HERE.
  	strategy	    ; 'forward, 'backward, 'BOD, or 'IN-LIT.
--- 2545,2556 ----
    ;;
    ;; The return value is a list, one of the following:
    ;;
!   ;; o - ('forward START-POINT) - scan forward from START-POINT,
!   ;;	 which is not less than the highest position in `c-state-cache' below here.
!   ;; o - ('backward nil) - scan backwards (from HERE).
!   ;; o - ('BOD START-POINT) - scan forwards from START-POINT, which is at the
    ;;   top level.
!   ;; o - ('IN-LIT nil) - point is inside the literal containing point-min.
    (let ((cache-pos (c-get-cache-scan-pos here))	; highest position below HERE in cache (or 1)
  	BOD-pos		    ; position of 2nd BOD before HERE.
  	strategy	    ; 'forward, 'backward, 'BOD, or 'IN-LIT.
***************
*** 2590,2596 ****
  
      (list
       strategy
-      (and (memq strategy '(forward backward)) cache-pos)
       (and (memq strategy '(forward BOD)) start-point))))

--- 2588,2593 ----
***************
*** 2657,2663 ****
    ;; reduce the time wasted in repeated fruitless searches in brace deserts.
    (save-excursion
      (save-restriction
!       (let* (new-cons
  	     (cache-pos (c-state-cache-top-lparen)) ; might be nil.
  	     (macro-start-or-from
  	      (progn (goto-char from)
--- 2654,2661 ----
    ;; reduce the time wasted in repeated fruitless searches in brace deserts.
    (save-excursion
      (save-restriction
!       (let* ((here (point-max))
! 	     new-cons
  	     (cache-pos (c-state-cache-top-lparen)) ; might be nil.
  	     (macro-start-or-from
  	      (progn (goto-char from)
***************
*** 2692,2697 ****
--- 2690,2696 ----
  		;; search bound, even though the algorithm below would skip
  		;; over the new paren pair.
  		(cache-lim (and cache-pos (< cache-pos from) cache-pos)))
+ 	    (widen)
  	    (narrow-to-region
  		(cond
  		 ((and desert-lim cache-lim)
***************
*** 2711,2726 ****
  		  (while
  		      (and (setq ce (scan-lists bra -1 -1)) ; back past )/]/}; might signal
  			   (setq bra (scan-lists ce -1 1)) ; back past (/[/{; might signal
! 			   (or (> ce upper-lim)
! 			       (not (eq (char-after bra) ?\{))
! 			       (and (goto-char bra)
! 				    (c-beginning-of-macro)
! 				    (< (point) macro-start-or-from))))))
  		(and ce (< ce bra)))
  	    (setq bra ce))	; If we just backed over an unbalanced closing
  					; brace, ignore it.
  
! 	  (if (and ce (< bra ce) (eq (char-after bra) ?\{))
  	      ;; We've found the desired brace-pair.
  	      (progn
  		(setq new-cons (cons bra (1+ ce)))
--- 2710,2727 ----
  		  (while
  		      (and (setq ce (scan-lists bra -1 -1)) ; back past )/]/}; might signal
  			   (setq bra (scan-lists ce -1 1)) ; back past (/[/{; might signal
! 			   (or (> bra here) ;(> ce here)
! 			       (and
! 				(< ce here)
! 				(or (not (eq (char-after bra) ?\{))
! 				    (and (goto-char bra)
! 					 (c-beginning-of-macro)
! 					 (< (point) macro-start-or-from))))))))
  		(and ce (< ce bra)))
  	    (setq bra ce))	; If we just backed over an unbalanced closing
  					; brace, ignore it.
  
! 	  (if (and ce (< ce here) (< bra ce) (eq (char-after bra) ?\{))
  	      ;; We've found the desired brace-pair.
  	      (progn
  		(setq new-cons (cons bra (1+ ce)))
***************
*** 2734,2740 ****
  		 (t (setq c-state-cache (cons new-cons c-state-cache)))))
  
  	    ;; We haven't found a brace pair.  Record this in the cache.
! 	    (setq c-state-brace-pair-desert (cons cache-pos from))))))))
  
  (defsubst c-state-push-any-brace-pair (bra+1 macro-start-or-here)
    ;; If BRA+1 is nil, do nothing.  Otherwise, BRA+1 is the buffer position
--- 2735,2745 ----
  		 (t (setq c-state-cache (cons new-cons c-state-cache)))))
  
  	    ;; We haven't found a brace pair.  Record this in the cache.
! 	    (setq c-state-brace-pair-desert
! 		  (cons (if (and ce (< bra ce) (> ce here)) ; {..} straddling HERE?
! 			    bra
! 			  (point-min))
! 			(min here from)))))))))
  
  (defsubst c-state-push-any-brace-pair (bra+1 macro-start-or-here)
    ;; If BRA+1 is nil, do nothing.  Otherwise, BRA+1 is the buffer position
***************
*** 2852,2870 ****
         (paren+1)
         (t from)))))
  
! (defun c-remove-stale-state-cache (good-pos pps-point)
    ;; Remove stale entries from the `c-cache-state', i.e. those which will
    ;; not be in it when it is amended for position (point-max).
    ;; Additionally, the "outermost" open-brace entry before (point-max)
    ;; will be converted to a cons if the matching close-brace is scanned.
    ;;
!   ;; GOOD-POS is a "maximal" "safe position" - there must be no open
!   ;; parens/braces/brackets between GOOD-POS and (point-max).
    ;;
    ;; As a second thing, calculate the result of parse-partial-sexp at
!   ;; PPS-POINT, w.r.t. GOOD-POS.  The motivation here is that
    ;; `c-state-cache-good-pos' may become PPS-POINT, but the caller may need to
!   ;; adjust it to get outside a string/comment.  (Sorry about this!  The code
    ;; needs to be FAST).
    ;;
    ;; Return a list (GOOD-POS SCAN-BACK-POS PPS-STATE), where
--- 2857,2875 ----
         (paren+1)
         (t from)))))
  
! (defun c-remove-stale-state-cache (start-point pps-point)
    ;; Remove stale entries from the `c-cache-state', i.e. those which will
    ;; not be in it when it is amended for position (point-max).
    ;; Additionally, the "outermost" open-brace entry before (point-max)
    ;; will be converted to a cons if the matching close-brace is scanned.
    ;;
!   ;; START-POINT is a "maximal" "safe position" - there must be no open
!   ;; parens/braces/brackets between START-POINT and (point-max).
    ;;
    ;; As a second thing, calculate the result of parse-partial-sexp at
!   ;; PPS-POINT, w.r.t. START-POINT.  The motivation here is that
    ;; `c-state-cache-good-pos' may become PPS-POINT, but the caller may need to
!   ;; adjust it to get outside a string/comment.	 (Sorry about this!  The code
    ;; needs to be FAST).
    ;;
    ;; Return a list (GOOD-POS SCAN-BACK-POS PPS-STATE), where
***************
*** 2872,2878 ****
    ;;   to be good (we aim for this to be as high as possible);
    ;; o - SCAN-BACK-POS, if not nil, indicates there may be a brace pair
    ;;   preceding POS which needs to be recorded in `c-state-cache'.  It is a
!   ;;   position to scan backwards from.
    ;; o - PPS-STATE is the parse-partial-sexp state at PPS-POINT.
    (save-restriction
      (narrow-to-region 1 (point-max))
--- 2877,2885 ----
    ;;   to be good (we aim for this to be as high as possible);
    ;; o - SCAN-BACK-POS, if not nil, indicates there may be a brace pair
    ;;   preceding POS which needs to be recorded in `c-state-cache'.  It is a
!   ;;   position to scan backwards from.  It is the position of the "{" of the
!   ;;   last element to be removed from `c-state-cache', when that elt is a
!   ;;   cons, otherwise nil.
    ;; o - PPS-STATE is the parse-partial-sexp state at PPS-POINT.
    (save-restriction
      (narrow-to-region 1 (point-max))
***************
*** 2882,2902 ****
  		(goto-char (point-max))
  		(and (c-beginning-of-macro)
  		     (point))))
! 	     (good-pos-actual-macro-start ; Start of macro containing good-pos
! 					; or nil
! 	      (and (< good-pos (point-max))
  		   (save-excursion
! 		     (goto-char good-pos)
  		     (and (c-beginning-of-macro)
  			  (point)))))
! 	     (good-pos-actual-macro-end	; End of this macro, (maybe
  					; (point-max)), or nil.
! 	      (and good-pos-actual-macro-start
  		   (save-excursion
! 		     (goto-char good-pos-actual-macro-start)
  		     (c-end-of-macro)
  		     (point))))
! 	     pps-state 			; Will be 9 or 10 elements long.
  	     pos
  	     upper-lim	   ; ,beyond which `c-state-cache' entries are removed
  	     scan-back-pos
--- 2889,2909 ----
  		(goto-char (point-max))
  		(and (c-beginning-of-macro)
  		     (point))))
! 	     (start-point-actual-macro-start ; Start of macro containing
! 					     ; start-point or nil
! 	      (and (< start-point (point-max))
  		   (save-excursion
! 		     (goto-char start-point)
  		     (and (c-beginning-of-macro)
  			  (point)))))
! 	     (start-point-actual-macro-end ; End of this macro, (maybe
  					; (point-max)), or nil.
! 	      (and start-point-actual-macro-start
  		   (save-excursion
! 		     (goto-char start-point-actual-macro-start)
  		     (c-end-of-macro)
  		     (point))))
! 	     pps-state			; Will be 9 or 10 elements long.
  	     pos
  	     upper-lim	   ; ,beyond which `c-state-cache' entries are removed
  	     scan-back-pos
***************
*** 2926,2940 ****
  	;; The next loop jumps forward out of a nested level of parens each
  	;; time round; the corresponding elements in `c-state-cache' are
  	;; removed.  `pos' is just after the brace-pair or the open paren at
! 	;; (car c-state-cache).  There can be no open parens/braces/brackets
! 	;; between `good-pos'/`good-pos-actual-macro-start' and (point-max),
  	;; due to the interface spec to this function.
! 	(setq pos (if (and good-pos-actual-macro-end
! 			   (not (eq good-pos-actual-macro-start
  				    in-macro-start)))
! 		      (1+ good-pos-actual-macro-end) ; get outside the macro as
  					; marked by a `category' text property.
! 		    good-pos))
  	(goto-char pos)
  	(while (and c-state-cache
  		    (< (point) (point-max)))
--- 2933,2947 ----
  	;; The next loop jumps forward out of a nested level of parens each
  	;; time round; the corresponding elements in `c-state-cache' are
  	;; removed.  `pos' is just after the brace-pair or the open paren at
! 	;; (car c-state-cache).	 There can be no open parens/braces/brackets
! 	;; between `start-point'/`start-point-actual-macro-start' and (point-max),
  	;; due to the interface spec to this function.
! 	(setq pos (if (and start-point-actual-macro-end
! 			   (not (eq start-point-actual-macro-start
  				    in-macro-start)))
! 		      (1+ start-point-actual-macro-end) ; get outside the macro as
  					; marked by a `category' text property.
! 		    start-point))
  	(goto-char pos)
  	(while (and c-state-cache
  		    (< (point) (point-max)))
***************
*** 2993,3006 ****
  
  	(list pos scan-back-pos pps-state)))))
  
! (defun c-remove-stale-state-cache-backwards (here cache-pos)
    ;; Strip stale elements of `c-state-cache' by moving backwards through the
    ;; buffer, and inform the caller of the scenario detected.
    ;;
    ;; HERE is the position we're setting `c-state-cache' for.
!   ;; CACHE-POS is just after the latest recorded position in `c-state-cache'
!   ;;   before HERE, or a position at or near point-min which isn't in a
!   ;;   literal.
    ;;
    ;; This function must only be called only when (> `c-state-cache-good-pos'
    ;; HERE).  Usually the gap between CACHE-POS and HERE is large.  It is thus
--- 3000,3013 ----
  
  	(list pos scan-back-pos pps-state)))))
  
! (defun c-remove-stale-state-cache-backwards (here)
    ;; Strip stale elements of `c-state-cache' by moving backwards through the
    ;; buffer, and inform the caller of the scenario detected.
    ;;
    ;; HERE is the position we're setting `c-state-cache' for.
!   ;; CACHE-POS (a locally bound variable) is just after the latest recorded
!   ;;   position in `c-state-cache' before HERE, or a position at or near
!   ;;   point-min which isn't in a literal.
    ;;
    ;; This function must only be called only when (> `c-state-cache-good-pos'
    ;; HERE).  Usually the gap between CACHE-POS and HERE is large.  It is thus
***************
*** 3023,3032 ****
    ;; The comments in this defun use "paren" to mean parenthesis or square
    ;; bracket (as contrasted with a brace), and "(" and ")" likewise.
    ;;
!   ;;    .   {..} (..) (..)  ( .. {   }  ) (...)    ( ....          .  ..)
!   ;;    |                   |       |   |     |                    |
!   ;;    CP                  E      here D     C                   good
!   (let ((pos c-state-cache-good-pos)
  	pa ren	       ; positions of "(" and ")"
  	dropped-cons ; whether the last element dropped from `c-state-cache'
  		     ; was a cons (representing a brace-pair)
--- 3030,3040 ----
    ;; The comments in this defun use "paren" to mean parenthesis or square
    ;; bracket (as contrasted with a brace), and "(" and ")" likewise.
    ;;
!   ;;	.   {..} (..) (..)  ( .. {   }	) (...)	   ( ....	   .  ..)
!   ;;	|		    |	    |	|     |			   |
!   ;;	CP		    E	   here D     C			  good
!   (let ((cache-pos (c-get-cache-scan-pos here))	; highest position below HERE in cache (or 1)
! 	(pos c-state-cache-good-pos)
  	pa ren	       ; positions of "(" and ")"
  	dropped-cons ; whether the last element dropped from `c-state-cache'
  		     ; was a cons (representing a brace-pair)
***************
*** 3263,3269 ****
  	 cache-pos	     ; highest position below HERE already existing in
  			     ; cache (or 1).
  	 good-pos
! 	 start-point
  	 bopl-state
  	 res
  	 scan-backward-pos scan-forward-p) ; used for 'backward.
--- 3271,3278 ----
  	 cache-pos	     ; highest position below HERE already existing in
  			     ; cache (or 1).
  	 good-pos
! 	 start-point ; (when scanning forward) a place below HERE where there
! 		     ; are no open parens/braces between it and HERE.
  	 bopl-state
  	 res
  	 scan-backward-pos scan-forward-p) ; used for 'backward.
***************
*** 3274,3281 ****
      ;; Strategy?
      (setq res (c-parse-state-get-strategy here c-state-cache-good-pos)
  	  strategy (car res)
! 	  cache-pos (cadr res)
! 	  start-point (nth 2 res))
  
      (when (eq strategy 'BOD)
        (setq c-state-cache nil
--- 3283,3289 ----
      ;; Strategy?
      (setq res (c-parse-state-get-strategy here c-state-cache-good-pos)
  	  strategy (car res)
! 	  start-point (cadr res))
  
      (when (eq strategy 'BOD)
        (setq c-state-cache nil
***************
*** 3302,3308 ****
  		good-pos)))
  
         ((eq strategy 'backward)
! 	(setq res (c-remove-stale-state-cache-backwards here cache-pos)
  	      good-pos (car res)
  	      scan-backward-pos (cadr res)
  	      scan-forward-p (car (cddr res)))
--- 3310,3316 ----
  		good-pos)))
  
         ((eq strategy 'backward)
! 	(setq res (c-remove-stale-state-cache-backwards here)
  	      good-pos (car res)
  	      scan-backward-pos (cadr res)
  	      scan-forward-p (car (cddr res)))

> -- 
> Michael Welsh Duggan
> ([email-masked])

-- 
Alan Mackenzie (Nuremberg, Germany).