bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 127 bytes --]

// This code has no variable declarations

void foo() {
    for (; (DWORD) a * b ;)
        ;

    for (; a * b ;)
        ;
}

[-- Attachment #2: fix-for.patch --]
[-- Type: text/plain, Size: 3799 bytes --]

=== modified file 'lisp/progmodes/cc-fonts.el'
--- lisp/progmodes/cc-fonts.el	2010-12-07 12:15:28 +0000
+++ lisp/progmodes/cc-fonts.el	2011-01-25 11:10:00 +0000
@@ -1080,7 +1080,8 @@
 	  ;; o - '<> if the arglist is of angle bracket type;
 	  ;; o - 'arglist if it's some other arglist;
 	  ;; o - nil, if not in an arglist at all.  This includes the
-	  ;;   parenthesised condition which follows "if", "while", etc.
+	  ;;   parenthesised condition which follows "if", "while", etc.,
+	  ;;   but not "for", which is 'arglist after `;'.
 	  context
 	  ;; The position of the next token after the closing paren of
 	  ;; the last detected cast.
@@ -1109,7 +1110,7 @@
 	  ;; `c-forward-decl-or-cast-1' and `c-forward-label' for
 	  ;; later fontification.
 	  (c-record-type-identifiers t)
-	  label-type
+	  label-type paren-state most-enclosing-brace
 	  c-record-ref-identifiers
 	  ;; Make `c-forward-type' calls mark up template arglists if
 	  ;; it finds any.  That's necessary so that we later will
@@ -1171,7 +1172,6 @@
 				 'font-lock-function-name-face))))
 	  (c-font-lock-function-postfix limit))
 
-	 
 	 (setq start-pos (point))
 	 (when
 	     ;; The result of the `if' condition below is true when we don't recognize a
@@ -1189,7 +1189,31 @@
 	    ;; (e.g. "for (").
 	    (let ((type (and (> match-pos (point-min))
 			     (c-get-char-property (1- match-pos) 'c-type))))
-	      (cond ((not (memq (char-before match-pos) '(?\( ?, ?\[ ?<)))
+	      (cond
+               (;; Try to not fontify the second and third clauses of
+		;; `for' statements as declarations.
+		(and (or (eq (char-before match-pos) ?\;)
+			 (save-excursion
+			   ;; Catch things like for(; (DWORD)(int) x &
+			   ;; y; ) without invoking the full might of
+			   ;; c-beginning-of-statement-1.
+			   (goto-char match-pos)
+			   (while (eq (char-before) ?\))
+			     (c-go-list-backward)
+			     (c-backward-syntactic-ws))
+			   (eq (char-before) ?\;)))
+		     
+                     (setq paren-state (c-parse-state))
+                     (setq most-enclosing-brace
+                           (c-most-enclosing-brace paren-state))
+		     (eq (char-after most-enclosing-brace) ?\())
+		
+                ;; After a ";" in a for-block. A declaration can never
+                ;; begin after a `;' if the most enclosing paren is a
+                ;; `('.
+		(setq context 'arglist
+                      c-restricted-<>-arglists t))
+               ((not (memq (char-before match-pos) '(?\( ?, ?\[ ?<)))
 		     (setq context nil
 			   c-restricted-<>-arglists nil))
 		    ;; A control flow expression
@@ -1252,7 +1276,7 @@
 		;; Are we at a declarator?  Try to go back to the declaration
 		;; to check this.  Note that `c-beginning-of-decl-1' is slow,
 		;; so we cache its result between calls.
-		(let (paren-state bod-res encl-pos is-typedef)
+		(let (bod-res encl-pos is-typedef)
 		  (goto-char start-pos)
 		  (save-excursion
 		    (unless (and decl-search-lim
@@ -1318,20 +1342,7 @@
 		;; Back up to the type to fontify the declarator(s).
 		(goto-char (car decl-or-cast))
 
-		(let ((decl-list
-		       (if context
-			   ;; Should normally not fontify a list of
-			   ;; declarators inside an arglist, but the first
-			   ;; argument in the ';' separated list of a "for"
-			   ;; statement is an exception.
-			   (when (eq (char-before match-pos) ?\()
-			     (save-excursion
-			       (goto-char (1- match-pos))
-			       (c-backward-syntactic-ws)
-			       (and (c-simple-skip-symbol-backward)
-				    (looking-at c-paren-stmt-key))))
-			 t)))
-
+                (let ((decl-list (not context)))
 		  ;; Fix the `c-decl-id-start' or `c-decl-type-start' property
 		  ;; before the first declarator if it's a list.
 		  ;; `c-font-lock-declarators' handles the rest.

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Lars Ingebrigtsen]

Daniel Colascione <dan.colascione@gmail.com> writes:

> // This code has no variable declarations
>
> void foo() {
>     for (; (DWORD) a * b ;)
>         ;
>
>     for (; a * b ;)
>         ;
> }
>

I can confirm that the Emacs trunk still highlights the "a" in these
examples wrong, and that Daniel's patch seems to fix the issue.
However, I'm totally unfamiliar with the cc-mode code, so it would be
nice if somebody could look at it before it's applied.

=== modified file 'lisp/progmodes/cc-fonts.el'
--- lisp/progmodes/cc-fonts.el	2010-12-07 12:15:28 +0000
+++ lisp/progmodes/cc-fonts.el	2011-01-25 11:10:00 +0000
@@ -1080,7 +1080,8 @@
 	  ;; o - '<> if the arglist is of angle bracket type;
 	  ;; o - 'arglist if it's some other arglist;
 	  ;; o - nil, if not in an arglist at all.  This includes the
-	  ;;   parenthesised condition which follows "if", "while", etc.
+	  ;;   parenthesised condition which follows "if", "while", etc.,
+	  ;;   but not "for", which is 'arglist after `;'.
 	  context
 	  ;; The position of the next token after the closing paren of
 	  ;; the last detected cast.
@@ -1109,7 +1110,7 @@
 	  ;; `c-forward-decl-or-cast-1' and `c-forward-label' for
 	  ;; later fontification.
 	  (c-record-type-identifiers t)
-	  label-type
+	  label-type paren-state most-enclosing-brace
 	  c-record-ref-identifiers
 	  ;; Make `c-forward-type' calls mark up template arglists if
 	  ;; it finds any.  That's necessary so that we later will
@@ -1171,7 +1172,6 @@
 				 'font-lock-function-name-face))))
 	  (c-font-lock-function-postfix limit))
 
-	 
 	 (setq start-pos (point))
 	 (when
 	     ;; The result of the `if' condition below is true when we don't recognize a
@@ -1189,7 +1189,31 @@
 	    ;; (e.g. "for (").
 	    (let ((type (and (> match-pos (point-min))
 			     (c-get-char-property (1- match-pos) 'c-type))))
-	      (cond ((not (memq (char-before match-pos) '(?\( ?, ?\[ ?<)))
+	      (cond
+               (;; Try to not fontify the second and third clauses of
+		;; `for' statements as declarations.
+		(and (or (eq (char-before match-pos) ?\;)
+			 (save-excursion
+			   ;; Catch things like for(; (DWORD)(int) x &
+			   ;; y; ) without invoking the full might of
+			   ;; c-beginning-of-statement-1.
+			   (goto-char match-pos)
+			   (while (eq (char-before) ?\))
+			     (c-go-list-backward)
+			     (c-backward-syntactic-ws))
+			   (eq (char-before) ?\;)))
+		     
+                     (setq paren-state (c-parse-state))
+                     (setq most-enclosing-brace
+                           (c-most-enclosing-brace paren-state))
+		     (eq (char-after most-enclosing-brace) ?\())
+		
+                ;; After a ";" in a for-block. A declaration can never
+                ;; begin after a `;' if the most enclosing paren is a
+                ;; `('.
+		(setq context 'arglist
+                      c-restricted-<>-arglists t))
+               ((not (memq (char-before match-pos) '(?\( ?, ?\[ ?<)))
 		     (setq context nil
 			   c-restricted-<>-arglists nil))
 		    ;; A control flow expression
@@ -1252,7 +1276,7 @@
 		;; Are we at a declarator?  Try to go back to the declaration
 		;; to check this.  Note that `c-beginning-of-decl-1' is slow,
 		;; so we cache its result between calls.
-		(let (paren-state bod-res encl-pos is-typedef)
+		(let (bod-res encl-pos is-typedef)
 		  (goto-char start-pos)
 		  (save-excursion
 		    (unless (and decl-search-lim
@@ -1318,20 +1342,7 @@
 		;; Back up to the type to fontify the declarator(s).
 		(goto-char (car decl-or-cast))
 
-		(let ((decl-list
-		       (if context
-			   ;; Should normally not fontify a list of
-			   ;; declarators inside an arglist, but the first
-			   ;; argument in the ';' separated list of a "for"
-			   ;; statement is an exception.
-			   (when (eq (char-before match-pos) ?\()
-			     (save-excursion
-			       (goto-char (1- match-pos))
-			       (c-backward-syntactic-ws)
-			       (and (c-simple-skip-symbol-backward)
-				    (looking-at c-paren-stmt-key))))
-			 t)))
-
+                (let ((decl-list (not context)))
 		  ;; Fix the `c-decl-id-start' or `c-decl-type-start' property
 		  ;; before the first declarator if it's a list.
 		  ;; `c-font-lock-declarators' handles the rest.



-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 407 bytes --]

On 02/25/2016 10:18 PM, Lars Ingebrigtsen wrote:
> +                ;; After a ";" in a for-block. A declaration can never
> +                ;; begin after a `;' if the most enclosing paren is a
> +                ;; `('.-declarators' handles the rest.

Hrm. That's not true in general. Consider Java's try-with-resources
construct:

    try (Foo foo = new Foo(); Bar bar = new Bar()) { ... }



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

On 02/25/2016 10:31 PM, Daniel Colascione wrote:
> On 02/25/2016 10:18 PM, Lars Ingebrigtsen wrote:
>> +                ;; After a ";" in a for-block. A declaration can never
>> +                ;; begin after a `;' if the most enclosing paren is a
>> +                ;; `('.-declarators' handles the rest.
> 
> Hrm. That's not true in general. Consider Java's try-with-resources
> construct:
> 
>     try (Foo foo = new Foo(); Bar bar = new Bar()) { ... }
> 

Gah, of course that's true in a `for' block specifically.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Alan Mackenzie]

Hello, Daniel and Lars.

On Fri, Feb 26, 2016 at 04:48:13PM +1030, Lars Ingebrigtsen wrote:
> Daniel Colascione <dan.colascione@gmail.com> writes:

> > // This code has no variable declarations
> >
> > void foo() {
> >     for (; (DWORD) a * b ;)
> >         ;
> >
> >     for (; a * b ;)
> >         ;
> > }
> >

> I can confirm that the Emacs trunk still highlights the "a" in these
> examples wrong, and that Daniel's patch seems to fix the issue.
> However, I'm totally unfamiliar with the cc-mode code, so it would be
> nice if somebody could look at it before it's applied.

OK.  I haven't actually tried this patch out, but there are things in it
I find concerning.

> === modified file 'lisp/progmodes/cc-fonts.el'
> --- lisp/progmodes/cc-fonts.el	2010-12-07 12:15:28 +0000
> +++ lisp/progmodes/cc-fonts.el	2011-01-25 11:10:00 +0000
> @@ -1080,7 +1080,8 @@
>  	  ;; o - '<> if the arglist is of angle bracket type;
>  	  ;; o - 'arglist if it's some other arglist;
>  	  ;; o - nil, if not in an arglist at all.  This includes the
> -	  ;;   parenthesised condition which follows "if", "while", etc.
> +	  ;;   parenthesised condition which follows "if", "while", etc.,
> +	  ;;   but not "for", which is 'arglist after `;'.

By what logic is `context' set to 'arglist in a "for" statement?  The
main function of `context' is to inform `c-forward-decl-or-cast-1' of
the context in which it is being called.

>  	  context
>  	  ;; The position of the next token after the closing paren of
>  	  ;; the last detected cast.
> @@ -1109,7 +1110,7 @@
>  	  ;; `c-forward-decl-or-cast-1' and `c-forward-label' for
>  	  ;; later fontification.
>  	  (c-record-type-identifiers t)
> -	  label-type
> +	  label-type paren-state most-enclosing-brace
>  	  c-record-ref-identifiers
>  	  ;; Make `c-forward-type' calls mark up template arglists if
>  	  ;; it finds any.  That's necessary so that we later will
> @@ -1171,7 +1172,6 @@
>  				 'font-lock-function-name-face))))
>  	  (c-font-lock-function-postfix limit))
 
> -	 
>  	 (setq start-pos (point))
>  	 (when
>  	     ;; The result of the `if' condition below is true when we don't recognize a

The next hunk attempts to move the detection of a "for" statement here
from later in the function where it previously was.  Why?

> @@ -1189,7 +1189,31 @@
>  	    ;; (e.g. "for (").
>  	    (let ((type (and (> match-pos (point-min))
>  			     (c-get-char-property (1- match-pos) 'c-type))))
> -	      (cond ((not (memq (char-before match-pos) '(?\( ?, ?\[ ?<)))
> +	      (cond
> +               (;; Try to not fontify the second and third clauses of
> +		;; `for' statements as declarations.
> +		(and (or (eq (char-before match-pos) ?\;)
> +			 (save-excursion
> +			   ;; Catch things like for(; (DWORD)(int) x &
> +			   ;; y; ) without invoking the full might of
> +			   ;; c-beginning-of-statement-1.
> +			   (goto-char match-pos)
> +			   (while (eq (char-before) ?\))
> +			     (c-go-list-backward)
> +			     (c-backward-syntactic-ws))

Here we potentially have an infinite loop when there's an unbalanced ")"
in the code.  It's critical to check the return from
`c-go-list-backward' here, too.

> +			   (eq (char-before) ?\;)))
> +		     
> +                     (setq paren-state (c-parse-state))
> +                     (setq most-enclosing-brace
> +                           (c-most-enclosing-brace paren-state))
> +		     (eq (char-after most-enclosing-brace) ?\())

Rather than using `c-parse-state', this could be done more efficiently
with `c-up-list-backward'.  There may be the possibility of an error
here if `c-most-enclosing-brace' returns nil, leading to (char-after
nil), but maybe that can't happen.  It would certainly be a good idea to
check for it, though.

> +		
> +                ;; After a ";" in a for-block. A declaration can never
> +                ;; begin after a `;' if the most enclosing paren is a
> +                ;; `('.

How do we know we're in a "for" block here?  There is no `looking-at'
check with the pertinent regexp (c-paren-stmt-key).

> +		(setq context 'arglist
> +                      c-restricted-<>-arglists t))

Again, why is `context' set to 'arglist here?  What effect is this
intended to have on `c-forward-decl-or-cast-1'?

> +               ((not (memq (char-before match-pos) '(?\( ?, ?\[ ?<)))
>  		     (setq context nil
>  			   c-restricted-<>-arglists nil))
>  		    ;; A control flow expression
> @@ -1252,7 +1276,7 @@
>  		;; Are we at a declarator?  Try to go back to the declaration
>  		;; to check this.  Note that `c-beginning-of-decl-1' is slow,
>  		;; so we cache its result between calls.
> -		(let (paren-state bod-res encl-pos is-typedef)
> +		(let (bod-res encl-pos is-typedef)
>  		  (goto-char start-pos)
>  		  (save-excursion
>  		    (unless (and decl-search-lim
> @@ -1318,20 +1342,7 @@
>  		;; Back up to the type to fontify the declarator(s).
>  		(goto-char (car decl-or-cast))
 
> -		(let ((decl-list
> -		       (if context
> -			   ;; Should normally not fontify a list of
> -			   ;; declarators inside an arglist, but the first
> -			   ;; argument in the ';' separated list of a "for"
> -			   ;; statement is an exception.
> -			   (when (eq (char-before match-pos) ?\()
> -			     (save-excursion
> -			       (goto-char (1- match-pos))
> -			       (c-backward-syntactic-ws)
> -			       (and (c-simple-skip-symbol-backward)
> -				    (looking-at c-paren-stmt-key))))
> -			 t)))
> -
> +                (let ((decl-list (not context)))

Here the setting of decl-list is changed.  Why?  `decl-list''s purpose
is to instruct `c-font-lock-declarators' whether to fontify just one
declarator or a whole list of them.  The existing logic is to fontify
all the declarators in a "for" block, whereas after the patch only the
first one would be fontified.  Again, why?

>  		  ;; Fix the `c-decl-id-start' or `c-decl-type-start' property
>  		  ;; before the first declarator if it's a list.
>  		  ;; `c-font-lock-declarators' handles the rest.

Question (for Daniel): has this patch been run through the CC Mode test
suite, yet?

> -- 
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Daniel Colascione]

[-- Attachment #1.1: Type: text/plain, Size: 6620 bytes --]

On 03/01/2016 10:02 AM, Alan Mackenzie wrote:
> Hello, Daniel and Lars.
> 
> On Fri, Feb 26, 2016 at 04:48:13PM +1030, Lars Ingebrigtsen wrote:
>> Daniel Colascione <dan.colascione@gmail.com> writes:
> 
>>> // This code has no variable declarations
>>>
>>> void foo() {
>>>     for (; (DWORD) a * b ;)
>>>         ;
>>>
>>>     for (; a * b ;)
>>>         ;
>>> }
>>>
> 
>> I can confirm that the Emacs trunk still highlights the "a" in these
>> examples wrong, and that Daniel's patch seems to fix the issue.
>> However, I'm totally unfamiliar with the cc-mode code, so it would be
>> nice if somebody could look at it before it's applied.
> 
> OK.  I haven't actually tried this patch out, but there are things in it
> I find concerning.
> 
>> === modified file 'lisp/progmodes/cc-fonts.el'
>> --- lisp/progmodes/cc-fonts.el	2010-12-07 12:15:28 +0000
>> +++ lisp/progmodes/cc-fonts.el	2011-01-25 11:10:00 +0000
>> @@ -1080,7 +1080,8 @@
>>  	  ;; o - '<> if the arglist is of angle bracket type;
>>  	  ;; o - 'arglist if it's some other arglist;
>>  	  ;; o - nil, if not in an arglist at all.  This includes the
>> -	  ;;   parenthesised condition which follows "if", "while", etc.
>> +	  ;;   parenthesised condition which follows "if", "while", etc.,
>> +	  ;;   but not "for", which is 'arglist after `;'.
> 
> By what logic is `context' set to 'arglist in a "for" statement?  The
> main function of `context' is to inform `c-forward-decl-or-cast-1' of
> the context in which it is being called.
> 
>>  	  context
>>  	  ;; The position of the next token after the closing paren of
>>  	  ;; the last detected cast.
>> @@ -1109,7 +1110,7 @@
>>  	  ;; `c-forward-decl-or-cast-1' and `c-forward-label' for
>>  	  ;; later fontification.
>>  	  (c-record-type-identifiers t)
>> -	  label-type
>> +	  label-type paren-state most-enclosing-brace
>>  	  c-record-ref-identifiers
>>  	  ;; Make `c-forward-type' calls mark up template arglists if
>>  	  ;; it finds any.  That's necessary so that we later will
>> @@ -1171,7 +1172,6 @@
>>  				 'font-lock-function-name-face))))
>>  	  (c-font-lock-function-postfix limit))
>  
>> -	 
>>  	 (setq start-pos (point))
>>  	 (when
>>  	     ;; The result of the `if' condition below is true when we don't recognize a
> 
> The next hunk attempts to move the detection of a "for" statement here
> from later in the function where it previously was.  Why?
> 
>> @@ -1189,7 +1189,31 @@
>>  	    ;; (e.g. "for (").
>>  	    (let ((type (and (> match-pos (point-min))
>>  			     (c-get-char-property (1- match-pos) 'c-type))))
>> -	      (cond ((not (memq (char-before match-pos) '(?\( ?, ?\[ ?<)))
>> +	      (cond
>> +               (;; Try to not fontify the second and third clauses of
>> +		;; `for' statements as declarations.
>> +		(and (or (eq (char-before match-pos) ?\;)
>> +			 (save-excursion
>> +			   ;; Catch things like for(; (DWORD)(int) x &
>> +			   ;; y; ) without invoking the full might of
>> +			   ;; c-beginning-of-statement-1.
>> +			   (goto-char match-pos)
>> +			   (while (eq (char-before) ?\))
>> +			     (c-go-list-backward)
>> +			     (c-backward-syntactic-ws))
> 
> Here we potentially have an infinite loop when there's an unbalanced ")"
> in the code.  It's critical to check the return from
> `c-go-list-backward' here, too.
> 
>> +			   (eq (char-before) ?\;)))
>> +		     
>> +                     (setq paren-state (c-parse-state))
>> +                     (setq most-enclosing-brace
>> +                           (c-most-enclosing-brace paren-state))
>> +		     (eq (char-after most-enclosing-brace) ?\())
> 
> Rather than using `c-parse-state', this could be done more efficiently
> with `c-up-list-backward'.  There may be the possibility of an error
> here if `c-most-enclosing-brace' returns nil, leading to (char-after
> nil), but maybe that can't happen.  It would certainly be a good idea to
> check for it, though.
> 
>> +		
>> +                ;; After a ";" in a for-block. A declaration can never
>> +                ;; begin after a `;' if the most enclosing paren is a
>> +                ;; `('.
> 
> How do we know we're in a "for" block here?  There is no `looking-at'
> check with the pertinent regexp (c-paren-stmt-key).
> 
>> +		(setq context 'arglist
>> +                      c-restricted-<>-arglists t))
> 
> Again, why is `context' set to 'arglist here?  What effect is this
> intended to have on `c-forward-decl-or-cast-1'?
> 
>> +               ((not (memq (char-before match-pos) '(?\( ?, ?\[ ?<)))
>>  		     (setq context nil
>>  			   c-restricted-<>-arglists nil))
>>  		    ;; A control flow expression
>> @@ -1252,7 +1276,7 @@
>>  		;; Are we at a declarator?  Try to go back to the declaration
>>  		;; to check this.  Note that `c-beginning-of-decl-1' is slow,
>>  		;; so we cache its result between calls.
>> -		(let (paren-state bod-res encl-pos is-typedef)
>> +		(let (bod-res encl-pos is-typedef)
>>  		  (goto-char start-pos)
>>  		  (save-excursion
>>  		    (unless (and decl-search-lim
>> @@ -1318,20 +1342,7 @@
>>  		;; Back up to the type to fontify the declarator(s).
>>  		(goto-char (car decl-or-cast))
>  
>> -		(let ((decl-list
>> -		       (if context
>> -			   ;; Should normally not fontify a list of
>> -			   ;; declarators inside an arglist, but the first
>> -			   ;; argument in the ';' separated list of a "for"
>> -			   ;; statement is an exception.
>> -			   (when (eq (char-before match-pos) ?\()
>> -			     (save-excursion
>> -			       (goto-char (1- match-pos))
>> -			       (c-backward-syntactic-ws)
>> -			       (and (c-simple-skip-symbol-backward)
>> -				    (looking-at c-paren-stmt-key))))
>> -			 t)))
>> -
>> +                (let ((decl-list (not context)))
> 
> Here the setting of decl-list is changed.  Why?  `decl-list''s purpose
> is to instruct `c-font-lock-declarators' whether to fontify just one
> declarator or a whole list of them.  The existing logic is to fontify
> all the declarators in a "for" block, whereas after the patch only the
> first one would be fontified.  Again, why?
> 
>>  		  ;; Fix the `c-decl-id-start' or `c-decl-type-start' property
>>  		  ;; before the first declarator if it's a list.
>>  		  ;; `c-font-lock-declarators' handles the rest.
> 
> Question (for Daniel): has this patch been run through the CC Mode test
> suite, yet?

It has not. It's been years since I even thought about that code. If
you're up for it, I'd rather you supply a separate fix.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Alan Mackenzie]

Hello, Daniel.

On Tue, Mar 01, 2016 at 10:05:47AM -0800, Daniel Colascione wrote:
> On 03/01/2016 10:02 AM, Alan Mackenzie wrote:

> > On Fri, Feb 26, 2016 at 04:48:13PM +1030, Lars Ingebrigtsen wrote:
> >> Daniel Colascione <dan.colascione@gmail.com> writes:

> >>> // This code has no variable declarations

> >>> void foo() {
> >>>     for (; (DWORD) a * b ;)
> >>>         ;

> >>>     for (; a * b ;)
> >>>         ;
> >>> }


> It's been years since I even thought about that code. If you're up for
> it, I'd rather you supply a separate fix.

OK, here goes:


diff -r f19a4ffb060b cc-fonts.el
--- a/cc-fonts.el	Fri Apr 01 12:23:17 2016 +0000
+++ b/cc-fonts.el	Fri Apr 01 16:10:57 2016 +0000
@@ -1206,8 +1206,20 @@
 			   'font-lock-keyword-face)
 		       (looking-at c-not-decl-init-keywords))
 		  (and c-macro-with-semi-re
-		       (looking-at c-macro-with-semi-re))) ; 2008-11-04
-	      ;; Don't do anything more if we're looking at a keyword that
+		       (looking-at c-macro-with-semi-re)) ; 2008-11-04
+		  (save-excursion ; A construct after a ; in a `for' statement
+				  ; can't be a declaration.
+		    (and (c-go-up-list-backward)
+			 (eq (char-after) ?\()
+			 (progn (c-backward-syntactic-ws)
+				(c-simple-skip-symbol-backward))
+			 (looking-at c-paren-stmt-key)
+			 (progn (goto-char match-pos)
+				(while (and (eq (char-before) ?\))
+					    (c-go-list-backward))
+				  (c-backward-syntactic-ws))
+				(eq (char-before) ?\;)))))
+	      ;; Don't do anything more if we're looking at something that
 	      ;; can't start a declaration.
 	      t
 

Could you do the usual with this patch, please, then if everything's OK,
I can commit it to the emacs-25 branch.  Thanks!

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Alan Mackenzie]

Bug fixed in master branch.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[npostavs]

found 7918 25.2
severity 7918 minor
tags 7918 fixed
close 7918 26.1
quit

> From: Alan Mackenzie <acm@muc.de>
> Subject: Re: bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations
> To: 7918-done@debbugs.gnu.org, 7918@debbugs.gnu.org
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, Daniel Colascione <dancol@dancol.org>
> Date: Mon, 25 Apr 2016 18:04:30 +0000 (1 year, 9 weeks, 1 day ago)
>
> Bug fixed in master branch.

I can confirm it's fixed in master (I wonder why the bug was reopened,
apparently automatically?)

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Glenn Morris]

npostavs@users.sourceforge.net wrote:

>> Bug fixed in master branch.
>
> I can confirm it's fixed in master (I wonder why the bug was reopened,
> apparently automatically?)

Nothing gets reopened automatically. I agree that sometimes debbugs
doesn't report the details of control requests in an informative way.
In this case, the bug was reopened in May 2016 (by Alan, from internal
logs). You'd have to ask him why.

http://lists.gnu.org/archive/html/emacs-bug-tracker/2016-05/msg00126.html

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[npostavs]

Glenn Morris <rgm@gnu.org> writes:

> npostavs@users.sourceforge.net wrote:
>
>>> Bug fixed in master branch.
>>
>> I can confirm it's fixed in master (I wonder why the bug was reopened,
>> apparently automatically?)
>
> Nothing gets reopened automatically. I agree that sometimes debbugs
> doesn't report the details of control requests in an informative way.
> In this case, the bug was reopened in May 2016 (by Alan, from internal
> logs). You'd have to ask him why.
>
> http://lists.gnu.org/archive/html/emacs-bug-tracker/2016-05/msg00126.html

Oh, I was looking at
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=7918;msg=32.  So all those
"fake control message" things are just the original control request
getting lost somehow?

Ccing Alan, please reopen if I made a mistake by closing this.

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Alan Mackenzie]

Hello, Noam and Glenn.

On Mon, Jul 03, 2017 at 15:46:00 -0400, npostavs@users.sourceforge.net wrote:
> Glenn Morris <rgm@gnu.org> writes:

> > npostavs@users.sourceforge.net wrote:

> >>> Bug fixed in master branch.

> >> I can confirm it's fixed in master (I wonder why the bug was reopened,
> >> apparently automatically?)

> > Nothing gets reopened automatically. I agree that sometimes debbugs
> > doesn't report the details of control requests in an informative way.
> > In this case, the bug was reopened in May 2016 (by Alan, from internal
> > logs). You'd have to ask him why.

It was because the original "fix" for the bug slowed CC Mode's
fontification down by a factor of ~3.

> > http://lists.gnu.org/archive/html/emacs-bug-tracker/2016-05/msg00126.html

> Oh, I was looking at
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=7918;msg=32.  So all those
> "fake control message" things are just the original control request
> getting lost somehow?

> Ccing Alan, please reopen if I made a mistake by closing this.

This is strange.  I sent an email attempting to close the bug for a
second time on 2016-05-16.  This email was actually received and
acknowledged by debbugs.gnu.org.  But apparently the bug didn't get
closed.  Maybe sending mail to 7918-done@debbugs.gnu.org only works the
first time around.

Anyway, it's fine for the bug finally to be marked closed.  Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Glenn Morris]

>> Oh, I was looking at
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=7918;msg=32.  So all those
>> "fake control message" things are just the original control request
>> getting lost somehow?

Yes, I think it's basically a debbugs bug.

> Maybe sending mail to 7918-done@debbugs.gnu.org only works the first
> time around.

Nope. There's no record of that second mail anywhere AFAICS.

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Alan Mackenzie]

Hello, Glenn.

On Wed, Jul 05, 2017 at 11:55:52 -0400, Glenn Morris wrote:

> >> Oh, I was looking at
> >> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=7918;msg=32.  So all those
> >> "fake control message" things are just the original control request
> >> getting lost somehow?

> Yes, I think it's basically a debbugs bug.

> > Maybe sending mail to 7918-done@debbugs.gnu.org only works the first
> > time around.

> Nope. There's no record of that second mail anywhere AFAICS.

Oh, but there is.  In my qmail log for 2016-05-16 appears the following
entry:

2016-05-16 11:38:16.645 +0000 new msg 4072519
2016-05-16 11:38:16.645 +0000 info msg 4072519: bytes 2479 from <acm@acm.muc.de> qp 19336 uid 1000
2016-05-16 11:38:16.719 +0000 starting delivery 73: msg 4072519 to remote 7918-done@debbugs.gnu.org
2016-05-16 11:38:16.719 +0000 status: local 0/10 remote 1/20
2016-05-16 11:38:17.974 +0000 delivery 73: success: 193.149.48.3_accepted_message./Remote_host_said:_250_Ok/
2016-05-16 11:38:17.975 +0000 status: local 0/10 remote 0/20
2016-05-16 11:38:17.975 +0000 end msg 4072519

This relates to my second message to 7918-done@debbugs.gnu.org, which was
clearly received by debbugs, just not acted upon.

Whether it's worth following up in any way is the question.  I'd be
inclined just to let the matter drop, unless the same thing happens again
at some stage.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Glenn Morris]

Alan Mackenzie wrote:

> 2016-05-16 11:38:16.645 +0000 new msg 4072519
> 2016-05-16 11:38:16.645 +0000 info msg 4072519: bytes 2479 from
> <acm@acm.muc.de> qp 19336 uid 1000
> 2016-05-16 11:38:16.719 +0000 starting delivery 73: msg 4072519 to
> remote 7918-done@debbugs.gnu.org
> 2016-05-16 11:38:16.719 +0000 status: local 0/10 remote 1/20
> 2016-05-16 11:38:17.974 +0000 delivery 73: success:
> 193.149.48.3_accepted_message./Remote_host_said:_250_Ok/

193.149.48.3 = mail.muc.de

> 2016-05-16 11:38:17.975 +0000 status: local 0/10 remote 0/20
> 2016-05-16 11:38:17.975 +0000 end msg 4072519
>
> This relates to my second message to 7918-done@debbugs.gnu.org, which was
> clearly received by debbugs, just not acted upon.

AFAICS there's nothing in the above that shows the mail was received by
debbugs. It was received by mail.muc.de, but then who knows...
There's no sign of it in any debbugs logs.

bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations

[Alan Mackenzie]

Hello, Glenn.

On Wed, Jul 05, 2017 at 21:39:23 -0400, Glenn Morris wrote:
> Alan Mackenzie wrote:

> > 2016-05-16 11:38:16.645 +0000 new msg 4072519
> > 2016-05-16 11:38:16.645 +0000 info msg 4072519: bytes 2479 from
> > <acm@acm.muc.de> qp 19336 uid 1000
> > 2016-05-16 11:38:16.719 +0000 starting delivery 73: msg 4072519 to
> > remote 7918-done@debbugs.gnu.org
> > 2016-05-16 11:38:16.719 +0000 status: local 0/10 remote 1/20
> > 2016-05-16 11:38:17.974 +0000 delivery 73: success:
> > 193.149.48.3_accepted_message./Remote_host_said:_250_Ok/

> 193.149.48.3 = mail.muc.de

> > 2016-05-16 11:38:17.975 +0000 status: local 0/10 remote 0/20
> > 2016-05-16 11:38:17.975 +0000 end msg 4072519
> >
> > This relates to my second message to 7918-done@debbugs.gnu.org, which was
> > clearly received by debbugs, just not acted upon.

> AFAICS there's nothing in the above that shows the mail was received by
> debbugs. It was received by mail.muc.de, but then who knows...
> There's no sign of it in any debbugs logs.

Yes, you're right.  Sorry about my misunderstanding.  All we can say is
that my email to 7918-done got lost somewhere between mail.muc.de and
debbugs.gnu.org.

-- 
Alan Mackenzie (Nuremberg, Germany).