Subject: Re: bug#59671: CC Mode 5.35.2 (C/*l); More incorrect type recognition

From: Alan Mackenzie <[email-masked]>
Date: Thu, 8 Dec 2022 19:00:04 +0000

Hello, Po.

On Tue, Nov 29, 2022 at 15:08:15 +0800, Po Lu via CC-Mode-help wrote:
> Package: cc-mode

> Insert the following code in a buffer:

> /* fns.c -- generic utility functions.  */

> /* Allocate a scratch buffer with the given size.  The buffer must be
>    used before the next call to allocscratch.  */

> static void *
> allocscratch (size_t size)
> {

> }

> now, on the line below the first comment, type:

> typedef struct foo

> then pause for a second, and type:

> bar;

> c-found-types will then contain "allocscratch"!.

Yes.  This is not good.  Please try out the attached patch which applies
cleanly to the release branch (and almost certainly to the master branch
too).  This should fix the problem, in that although "allocscratch" gets
entered into c-found-types, it gets removed again as soon as a type and
declarator are typed in after the typedef.

> Emacs  : GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu)
>  of 2022-11-29
> Package: CC Mode 5.35.2 (C/*l)
> Buffer Style: gnu
> c-emacs-features: (pps-extended-state col-0-paren posix-char-classes gen-string-delim gen-comment-delim syntax-properties category-properties 1-bit)

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

diff --git a/lisp/progmodes/cc-defs.el b/lisp/progmodes/cc-defs.el
index b13f6a5914..759620c533 100644
--- a/lisp/progmodes/cc-defs.el
+++ b/lisp/progmodes/cc-defs.el
@@ -1361,6 +1361,28 @@ c-search-forward-char-property
        (search-forward-regexp "\\(\n\\|.\\)")	; to set the match-data.
        (point))))
 
+(defmacro c-search-forward-non-nil-char-property (property &optional limit)
+  "Search forward for a text-property PROPERTY value non-nil.
+LIMIT bounds the search.  The comparison is done with `equal'.
+
+Leave point just after the character.  The match data remain
+unchanged.  Return the value of PROPERTY.  If a non-nil value
+isn't found, return nil; point is then left undefined."
+  (declare (debug t))
+  `(let* ((-limit- (or ,limit (point-max)))
+	  (value (c-get-char-property (point) ,property)))
+     (cond
+      ((>= (point) -limit-)
+       nil)
+      (value
+       (forward-char)
+       value)
+      (t (let ((place (c-next-single-property-change
+		       (point) ,property nil -limit-)))
+	   (when place
+	     (goto-char (1+ place))
+	     (c-get-char-property place ,property)))))))
+
 (defmacro c-search-backward-char-property (property value &optional limit)
   "Search backward for a text-property PROPERTY having value VALUE.
 LIMIT bounds the search.  The comparison is done with `equal'.
diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index edb873f5a6..115fcfa20a 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -142,6 +142,10 @@
 ;;       Put on the brace which introduces a brace list and on the commas
 ;;       which separate the elements within it.
 ;;
+;; 'c-typedef This property is applied to the first character of a
+;;   "typedef" keyword.  It's value is a list of the identifiers that
+;;   the "typedef" declares as types.
+;;
 ;; 'c-<>-c-types-set
 ;;   This property is set on an opening angle bracket, and indicates that
 ;;   any "," separators within the template/generic expression have been
@@ -10024,10 +10028,10 @@ c-fdoc-shift-type-backward
   ;; an identifier instead.
   (declare (debug nil))
   `(progn
+     (setq identifier-start type-start)
      ,(unless short
 	;; These identifiers are bound only in the inner let.
 	'(setq identifier-type at-type
-	       identifier-start type-start
 	       got-parens nil
 	       got-identifier t
 	       got-suffix t
@@ -10102,10 +10106,11 @@ c-forward-decl-or-cast-1
   ;;   The second element of the return value is non-nil when something
   ;;   indicating the identifier is a type occurs in the declaration.
   ;;   Specifically it is nil, or a three element list (A B C) where C is t
-  ;;   when context is '<> and the "identifier" is a found type, B is t when a
-  ;;   `c-typedef-kwds' ("typedef") is present, and A is t when some other
-  ;;   `c-typedef-decl-kwds' (e.g. class, struct, enum) specifier is present.
-  ;;   I.e., (some of) the declared identifier(s) are types.
+  ;;   when context is '<> and the "identifier" is a found type, B is the
+  ;;   position of the `c-typedef-kwds' keyword ("typedef") when such is
+  ;;   present, and A is t when some other `c-typedef-decl-kwds' (e.g. class,
+  ;;   struct, enum) specifier is present.  I.e., (some of) the declared
+  ;;   identifier(s) are types.
   ;;
   ;;   The third element of the return value is non-nil when the declaration
   ;;   parsed might be an expression.  The fourth element is the position of
@@ -10173,6 +10178,9 @@ c-forward-decl-or-cast-1
 	;; `c-decl-hangon-kwds' and their associated clauses that
 	;; occurs after the type.
 	id-start
+	;; The earlier value of `type-start' if we've shifted the type
+	;; backwards.
+	identifier-start
 	;; These store `at-type', `type-start' and `id-start' of the
 	;; identifier before the one in those variables.  The previous
 	;; identifier might turn out to be the real type in a
@@ -10183,7 +10191,8 @@ c-forward-decl-or-cast-1
 	;; Set if we've found a specifier (apart from "typedef") that makes
 	;; the defined identifier(s) types.
 	at-type-decl
-	;; Set if we've a "typedef" keyword.
+	;; If we've a "typedef" keyword (?or similar), the buffer position of
+	;; its first character.
 	at-typedef
 	;; Set if `context' is '<> and the identifier is definitely a type, or
 	;; has already been recorded as a found type.
@@ -10266,7 +10275,7 @@ c-forward-decl-or-cast-1
 		 (looking-at "@[A-Za-z0-9]+")))
 	    (save-match-data
 	      (if (looking-at c-typedef-key)
-		  (setq at-typedef t)))
+		  (setq at-typedef (point))))
 	    (setq kwd-sym (c-keyword-sym (match-string 1)))
 	    (save-excursion
 	      (c-forward-keyword-clause 1)
@@ -10486,9 +10495,9 @@ c-forward-decl-or-cast-1
 	  ;; True if we've parsed the type decl to a token that is
 	  ;; known to end declarations in this context.
 	  at-decl-end
-	  ;; The earlier values of `at-type' and `type-start' if we've
-	  ;; shifted the type backwards.
-	  identifier-type identifier-start
+	  ;; The earlier value of `at-type' if we've shifted the type
+	  ;; backwards.
+	  identifier-type
 	  ;; If `c-parse-and-markup-<>-arglists' is set we need to
 	  ;; turn it off during the name skipping below to avoid
 	  ;; getting `c-type' properties that might be bogus.  That
@@ -10530,6 +10539,10 @@ c-forward-decl-or-cast-1
 			      (progn (setq got-identifier nil) t)
 			    ;; It turned out to be the real identifier,
 			    ;; so stop.
+			    (save-excursion
+			      (c-backward-syntactic-ws)
+			      (c-simple-skip-symbol-backward)
+			      (setq identifier-start (point)))
 			    nil))
 		      t))
 
@@ -10555,6 +10568,10 @@ c-forward-decl-or-cast-1
 	  (and (looking-at c-identifier-start)
 	       (setq pos (point))
 	       (setq got-identifier (c-forward-name))
+	       (save-excursion
+		 (c-backward-syntactic-ws)
+		 (c-simple-skip-symbol-backward)
+		 (setq identifier-start (point)))
 	       (setq name-start pos))
 	  (when (looking-at "[0-9]")
 	    (setq got-number t)) ; We probably have an arithmetic expression.
@@ -10573,7 +10590,8 @@ c-forward-decl-or-cast-1
 	       (setq at-type nil
 		     name-start type-start
 		     id-start type-start
-		     got-identifier t)))
+		     got-identifier t)
+	       (setq identifier-start type-start)))
 
       ;; Skip over type decl suffix operators and trailing noise macros.
       (while
diff --git a/lisp/progmodes/cc-fonts.el b/lisp/progmodes/cc-fonts.el
index 9444828a0e..13eba2b3f6 100644
--- a/lisp/progmodes/cc-fonts.el
+++ b/lisp/progmodes/cc-fonts.el
@@ -1109,10 +1109,12 @@ c-font-lock-declarators
   ;; additionally, mark the commas with c-type property 'c-decl-id-start or
   ;; 'c-decl-type-start (according to TYPES).  Stop at LIMIT.
   ;;
-  ;; If TYPES is t, fontify all identifiers as types, if it is nil fontify as
-  ;; either variables or functions, otherwise TYPES is a face to use.  If
-  ;; NOT-TOP is non-nil, we are not at the top-level ("top-level" includes
-  ;; being directly inside a class or namespace, etc.).
+  ;; If TYPES is t, fontify all identifiers as types; if it is a number, a
+  ;; buffer position, additionally set the `c-deftype' text property on the
+  ;; keyword at that position; if it is nil fontify as either variables or
+  ;; functions, otherwise TYPES is a face to use.  If NOT-TOP is non-nil, we
+  ;; are not at the top-level ("top-level" includes being directly inside a
+  ;; class or namespace, etc.).
   ;;
   ;; TEMPLATE-CLASS is non-nil when the declaration is in template delimiters
   ;; and was introduced by, e.g. "typename" or "class", such that if there is
@@ -1129,17 +1131,28 @@ c-font-lock-declarators
   ;;(message "c-font-lock-declarators from %s to %s" (point) limit)
   (c-fontify-types-and-refs
       ()
+    ;; If we're altering the declarators in a typedef, we need to scan ALL of
+    ;; them because of the way we check for changes.
+    (let ((c-do-decl-limit (if (numberp types) (point-max) limit))
+	  decl-ids)
     (c-do-declarators
-     limit list not-top
-     (cond ((eq types t) 'c-decl-type-start)
+     c-do-decl-limit
+     list not-top
+     (cond ((or (numberp types)
+		(eq types t))
+	    'c-decl-type-start)
 	   ((null types) 'c-decl-id-start))
      (lambda (id-start id-end end-pos _not-top is-function init-char)
-       (if (eq types t)
+       (if (or (numberp types)
+	       (eq types t))
 	   (when id-start
 	     ;; Register and fontify the identifier as a type.
 	     (let ((c-promote-possible-types t))
 	       (goto-char id-start)
-	       (c-forward-type)))
+	       (c-forward-type))
+	     (when (numberp types)
+	       (push (buffer-substring-no-properties id-start id-end)
+		     decl-ids)))
 	 (when id-start
 	   (goto-char id-start)
 	   (when c-opt-identifier-prefix-key
@@ -1147,7 +1160,7 @@ c-font-lock-declarators
 			  (eq (match-end 1) id-end))
 	       (while (and (< (point) id-end)
 			   (re-search-forward c-opt-identifier-prefix-key id-end t))
-		 (c-forward-syntactic-ws limit))))
+		 (c-forward-syntactic-ws c-do-decl-limit))))
 	   ;; Only apply the face when the text doesn't have one yet.
 	   ;; Exception: The "" in C++'s operator"" will already wrongly have
 	   ;; string face.
@@ -1164,7 +1177,7 @@ c-font-lock-declarators
 		  (equal (buffer-substring-no-properties id-start id-end)
 			 "\"\""))
 	     (goto-char id-end)
-	     (c-forward-syntactic-ws limit)
+	     (c-forward-syntactic-ws c-do-decl-limit)
 	     (when (c-on-identifier)
 	       (c-put-font-lock-face
 		(point)
@@ -1174,10 +1187,21 @@ c-font-lock-declarators
 	    (eq init-char ?=)		; C++ "<class X = Y>"?
 	    (progn
 	      (goto-char end-pos)
-	      (c-forward-token-2 1 nil limit) ; Over "="
+	      (c-forward-token-2 1 nil c-do-decl-limit) ; Over "="
 	      (let ((c-promote-possible-types t))
 		(c-forward-type t)))))
      accept-anon)			; Last argument to c-do-declarators.
+    ;; If we've changed types declared by a "typedef", update the `c-typedef'
+    ;; text property.
+    (when (numberp types)
+      (let* ((old-decl-ids (c-get-char-property types 'c-typedef))
+	     (old-types (c--set-difference old-decl-ids decl-ids :test #'equal))
+	     (new-types (c--set-difference decl-ids old-decl-ids :test #'equal)))
+	(dolist (type old-types)
+	  (c-unfind-type type))
+	;; The new types have already been added to `c-found-types', as needed.
+	(when (or old-types new-types)
+	  (c-put-char-property types 'c-typedef decl-ids)))))
     nil))
 
 (defun c-get-fontification-context (match-pos not-front-decl &optional toplev)
@@ -1433,7 +1457,10 @@ c-font-lock-single-decl
       (c-font-lock-declarators
        (min limit (point-max))
        decl-list
-       (not (null (cadr decl-or-cast)))
+       (cond ((null (cadr decl-or-cast))
+	      nil)
+	     ((cadr (cadr decl-or-cast)))
+	     (t t))
        (not toplev)
        template-class
        (memq context '(decl <>))))
@@ -1730,6 +1757,40 @@ c-font-lock-enum-tail
 	(c-font-lock-declarators limit t nil t))))
   nil)
 
+(defun c-font-lock-enclosing-decls (limit)
+  ;; Fontify the declarators of (nested) declarations we're in the middle of.
+  ;; This is mainly for when a jit-lock etc. chunk starts inside the brace
+  ;; block of a struct/union/class, etc.
+  ;;
+  ;; This function will be called from font-lock for a region bounded by POINT
+  ;; and LIMIT, as though it were to identify a keyword for
+  ;; font-lock-keyword-face.  It always returns NIL to inhibit this and
+  ;; prevent a repeat invocation.  See elisp/lispref page "Search-based
+  ;; Fontification".
+  (c-skip-comments-and-strings limit)
+  (when (< (point) limit)
+    (let* ((paren-state (c-parse-state))
+	   (decl-search-lim (c-determine-limit 1000))
+	   in-typedef ps-elt)
+      ;; Are we in any nested struct/union/class/etc. braces?
+      (while paren-state
+	(setq ps-elt (car paren-state)
+	      paren-state (cdr paren-state))
+	(when (and (atom ps-elt)
+		   (eq (char-after ps-elt) ?\{))
+	  (goto-char ps-elt)
+	  (c-syntactic-skip-backward "^;{}" decl-search-lim)
+	  (c-forward-syntactic-ws)
+	  (setq in-typedef (looking-at c-typedef-key))
+	  (if in-typedef (c-forward-over-token-and-ws))
+	  (when (and c-opt-block-decls-with-vars-key
+		     (looking-at c-opt-block-decls-with-vars-key))
+	    (goto-char ps-elt)
+	    (when (c-safe (c-forward-sexp))
+	      (c-forward-syntactic-ws)
+	      (c-font-lock-declarators limit t in-typedef
+				       (not (c-bs-at-toplevel-p (point)))))))))))
+
 (defun c-font-lock-cut-off-declarators (limit)
   ;; Fontify any declarators "cut off" from their declaring type at the start
   ;; of the region being fontified.
@@ -1749,12 +1810,21 @@ c-font-lock-cut-off-declarators
 					; speeds up lisp.h tremendously.
       (save-excursion
 	(when (not (c-back-over-member-initializers decl-search-lim))
+	  (setq paren-state (c-parse-state))
 	  (unless (or (eobp)
 		      (looking-at "\\s(\\|\\s)"))
 	    (forward-char))
 	  (c-syntactic-skip-backward "^;{}" decl-search-lim t)
-	  (when (eq (char-before) ?})
-	    (c-go-list-backward)	; brace block of struct, etc.?
+	  ;; Do we have the brace block of a struct, etc.?
+	  (when (cond
+		 ((and (consp (car paren-state))
+		       (eq (char-before) ?}))
+		  (goto-char (caar paren-state))
+		  t)
+		 ((and (numberp (car paren-state))
+		       (eq (char-after (car paren-state)) ?{))
+		  (goto-char (car paren-state))
+		  t))
 	    (c-syntactic-skip-backward "^;{}" decl-search-lim t))
 	  (when (or (bobp)
 		    (memq (char-before) '(?\; ?{ ?})))
@@ -1782,40 +1852,6 @@ c-font-lock-cut-off-declarators
 					       context top-level))))))))
       nil)))
 
-(defun c-font-lock-enclosing-decls (limit)
-  ;; Fontify the declarators of (nested) declarations we're in the middle of.
-  ;; This is mainly for when a jit-lock etc. chunk starts inside the brace
-  ;; block of a struct/union/class, etc.
-  ;;
-  ;; This function will be called from font-lock for a region bounded by POINT
-  ;; and LIMIT, as though it were to identify a keyword for
-  ;; font-lock-keyword-face.  It always returns NIL to inhibit this and
-  ;; prevent a repeat invocation.  See elisp/lispref page "Search-based
-  ;; Fontification".
-  (c-skip-comments-and-strings limit)
-  (when (< (point) limit)
-    (let* ((paren-state (c-parse-state))
-	   (decl-search-lim (c-determine-limit 1000))
-	   in-typedef ps-elt)
-      ;; Are we in any nested struct/union/class/etc. braces?
-      (while paren-state
-	(setq ps-elt (car paren-state)
-	      paren-state (cdr paren-state))
-	(when (and (atom ps-elt)
-		   (eq (char-after ps-elt) ?\{))
-	  (goto-char ps-elt)
-	  (c-syntactic-skip-backward "^;{}" decl-search-lim)
-	  (c-forward-syntactic-ws)
-	  (setq in-typedef (looking-at c-typedef-key))
-	  (if in-typedef (c-forward-over-token-and-ws))
-	  (when (and c-opt-block-decls-with-vars-key
-		     (looking-at c-opt-block-decls-with-vars-key))
-	    (goto-char ps-elt)
-	    (when (c-safe (c-forward-sexp))
-	      (c-forward-syntactic-ws)
-	      (c-font-lock-declarators limit t in-typedef
-				       (not (c-bs-at-toplevel-p (point)))))))))))
-
 (defun c-font-lock-ml-strings (limit)
   ;; Fontify multi-line strings.
   ;;
diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 6a2c2f2911..74b493d24a 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -2077,6 +2077,37 @@ c-after-change-fix-comment-escapes
 		 (not (eobp))))
 	(forward-char))))))
 
+(defun c-before-change-de-typedef (beg end)
+  ;; For each "typedef" starting in (BEG END), remove the defined types from
+  ;; c-found-types
+  (let (prop)
+    (save-excursion
+      (goto-char beg)
+      (while (and (< (point) end)
+		  (setq prop (c-search-forward-non-nil-char-property
+			      'c-typedef)))
+	(dolist (type prop)
+	  (c-unfind-type type))))))
+
+(defun c-after-change-de-typedef (beg end _old-len)
+  ;; For each former "typedef" in (BEG END), remove the defined types from
+  ;; those which are no longer typedefs.
+  (let (prop)
+    (save-excursion
+      (goto-char beg)
+      (c-backward-token-2
+       1 nil (- (point) 20))
+      (while (and (< (point) end)
+		  (setq prop (c-search-forward-non-nil-char-property
+			      'c-typedef end)))
+	(backward-char)
+	(when (or (not (looking-at c-typedef-key))
+		  (<= (match-end 1) beg))
+	  (dolist (type prop)
+	    (c-unfind-type type))
+	  (c-clear-char-property (point) 'c-typedef))
+	(forward-char)))))
+
 (defun c-update-new-id (end)
   ;; Note the bounds of any identifier that END is in or just after, in
   ;; `c-new-id-start' and `c-new-id-end'.  Otherwise set these variables to
@@ -2215,6 +2246,10 @@ c-before-change
 							       term-pos)
 			       (buffer-substring-no-properties beg end)))))))
 
+	   ;; If we're about to delete "typedef"s, clear the identifiers from
+	   ;; `c-found-types'.
+	   (c-before-change-de-typedef beg end)
+
 	   (if c-get-state-before-change-functions
 	       (mapc (lambda (fn)
 		       (funcall fn beg end))
@@ -2304,11 +2339,11 @@ c-after-change
 	       (c-clear-char-property-with-value beg end 'syntax-table nil)))
 
 	   (c-update-new-id end)
-	   (c-trim-found-types beg end old-len) ; maybe we don't
-					; need all of these.
+	   (c-trim-found-types beg end old-len) ; maybe we don't need all
+					; of these.
+	   (c-after-change-de-typedef beg end old-len)
 	   (c-invalidate-sws-region-after beg end old-len)
-	   ;; (c-invalidate-state-cache beg) ; moved to
-	   ;; `c-before-change'.
+	   ;; (c-invalidate-state-cache beg) ; moved to `c-before-change'.
 	   (c-invalidate-find-decl-cache beg)
 
 	   (when c-recognize-<>-arglists