From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Daniel Colascione Newsgroups: gmane.emacs.bugs Subject: bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations Date: Tue, 1 Mar 2016 10:05:47 -0800 Message-ID: <56D5D9FB.2060605@dancol.org> References: <87k2lsuh96.fsf@gnus.org> <20160301180242.GA6494@acm.fritz.box> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="NvtM4XpaobQad1NKvQAmUaKoucx85uD7O" X-Trace: ger.gmane.org 1456855593 16962 80.91.229.3 (1 Mar 2016 18:06:33 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 1 Mar 2016 18:06:33 +0000 (UTC) Cc: 7918@debbugs.gnu.org To: Alan Mackenzie , Daniel Colascione , Lars Ingebrigtsen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Mar 01 19:06:19 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1aaogZ-0005uH-P2 for geb-bug-gnu-emacs@m.gmane.org; Tue, 01 Mar 2016 19:06:16 +0100 Original-Received: from localhost ([::1]:51561 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaogZ-0004xT-7s for geb-bug-gnu-emacs@m.gmane.org; Tue, 01 Mar 2016 13:06:15 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:34633) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaogU-0004ve-UM for bug-gnu-emacs@gnu.org; Tue, 01 Mar 2016 13:06:12 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaogT-00071c-Gn for bug-gnu-emacs@gnu.org; Tue, 01 Mar 2016 13:06:10 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:59523) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaogM-0006zP-JX; Tue, 01 Mar 2016 13:06:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84) (envelope-from ) id 1aaogM-0001iA-EV; Tue, 01 Mar 2016 13:06:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Daniel Colascione Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org, bug-cc-mode@gnu.org Resent-Date: Tue, 01 Mar 2016 18:06:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 7918 X-GNU-PR-Package: emacs,cc-mode X-GNU-PR-Keywords: patch confirmed Original-Received: via spool by 7918-submit@debbugs.gnu.org id=B7918.14568555576549 (code B ref 7918); Tue, 01 Mar 2016 18:06:02 +0000 Original-Received: (at 7918) by debbugs.gnu.org; 1 Mar 2016 18:05:57 +0000 Original-Received: from localhost ([127.0.0.1]:56646 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84) (envelope-from ) id 1aaogH-0001hY-Fu for submit@debbugs.gnu.org; Tue, 01 Mar 2016 13:05:57 -0500 Original-Received: from dancol.org ([96.126.100.184]:34226) by debbugs.gnu.org with esmtp (Exim 4.84) (envelope-from ) id 1aaogF-0001hP-Sz for 7918@debbugs.gnu.org; Tue, 01 Mar 2016 13:05:56 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=dancol.org; s=x; h=Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:From:Cc:References:To:Subject; bh=2ewgsoiSgf+tZy4LmRW5J8U5LhvRKiZx9jbaCFagC/c=; b=rUMy4UcQ4qr1uxQZ9xVPHpAPmtGACxfn1Jo9VdZqb2Ge/FJOWC+7FHoD7Ow5xweMsrPg0EW6+X7NqRZXjRJ8fBEFRzyXPSg58y5l2RvAZpGRoh3geeRnkMWXFfRlllLb0+i3Hs3lEkj1LRF4q5cu3QVqnIBDIi+v1Y/LIpMQQf5LjqRoBdOIUYIjB9Ch/PfbsTsrABS22x/PuxMQowz9sR2Utbzrgp2F6rWieDiQjc449uo0lKk1mU1Zy+hfkQ1pkngd5EUcXnzU6JMWpcQGimwrkp/30CGd6nYh4rbrKyN2nr7EDUxodmas/tXiyWTX6PwHDU45P3o9sDtPbiLH9A==; Original-Received: from [2620:10d:c090:180::1:ab21] (helo=[IPv6:2620:10d:c081:1103:2ab2:bdff:fe1c:db58]) by dancol.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84) (envelope-from ) id 1aaogD-00047y-Gr; Tue, 01 Mar 2016 10:05:53 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 In-Reply-To: <20160301180242.GA6494@acm.fritz.box> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:114268 Archived-At: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --NvtM4XpaobQad1NKvQAmUaKoucx85uD7O Content-Type: multipart/mixed; boundary="dQ76rfjFgvElWgpjSWFHPgCDeXlvugKIo" From: Daniel Colascione To: Alan Mackenzie , Daniel Colascione , Lars Ingebrigtsen Cc: 7918@debbugs.gnu.org Message-ID: <56D5D9FB.2060605@dancol.org> Subject: Re: bug#7918: [PATCH] cc-mode: only the first clause of a for-loop should be checked for declarations References: <87k2lsuh96.fsf@gnus.org> <20160301180242.GA6494@acm.fritz.box> In-Reply-To: <20160301180242.GA6494@acm.fritz.box> --dQ76rfjFgvElWgpjSWFHPgCDeXlvugKIo Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 03/01/2016 10:02 AM, Alan Mackenzie wrote: > Hello, Daniel and Lars. >=20 > On Fri, Feb 26, 2016 at 04:48:13PM +1030, Lars Ingebrigtsen wrote: >> Daniel Colascione writes: >=20 >>> // This code has no variable declarations >>> >>> void foo() { >>> for (; (DWORD) a * b ;) >>> ; >>> >>> for (; a * b ;) >>> ; >>> } >>> >=20 >> 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. >=20 > OK. I haven't actually tried this patch out, but there are things in i= t > I find concerning. >=20 >> =3D=3D=3D 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 `;'. >=20 > 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. >=20 >> 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)) > =20 >> - =20 >> (setq start-pos (point)) >> (when >> ;; The result of the `if' condition below is true when we don't= recognize a >=20 > The next hunk attempts to move the detection of a "for" statement here > from later in the function where it previously was. Why? >=20 >> @@ -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)) >=20 > 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. >=20 >> + (eq (char-before) ?\;))) >> + =20 >> + (setq paren-state (c-parse-state)) >> + (setq most-enclosing-brace >> + (c-most-enclosing-brace paren-state)) >> + (eq (char-after most-enclosing-brace) ?\()) >=20 > 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 t= o > check for it, though. >=20 >> + =09 >> + ;; After a ";" in a for-block. A declaration can neve= r >> + ;; begin after a `;' if the most enclosing paren is a= >> + ;; `('. >=20 > 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). >=20 >> + (setq context 'arglist >> + c-restricted-<>-arglists t)) >=20 > Again, why is `context' set to 'arglist here? What effect is this > intended to have on `c-forward-decl-or-cast-1'? >=20 >> + ((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)) > =20 >> - (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))) >=20 > 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? >=20 >> ;; 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. >=20 > 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. --dQ76rfjFgvElWgpjSWFHPgCDeXlvugKIo-- --NvtM4XpaobQad1NKvQAmUaKoucx85uD7O Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJW1dn7AAoJEN4WImmbpWBllTgQAJdL82VguoGrU/9W5criX8wH 06Ku8/5waccnefy5g4UBLTaRRwbUDB+E9vsPwOVgpcbN+WUL3GpJQbOb3hpsUDtl cJTuUbPsrtR79fIvYB8WOD0a68tI9ED6cv09U1bXcu3swE/WTiyD8KDHOQfB4CBa dlK1P6fOsGa3WWLy3JCy1OV4/7flD7jtGMuGkSJ3/odxU21n8gtgbweTdpLdRM4u CcViwV7M0YVKd6A1ngWlkZdsaN4jzIQp3qgXAf5CFl/M/CiT7tM+OVsapkPPzrJY Fh24wjnhd2moRkgrXEQChl16C8t8K89Hj+MildDjLWquwKpYskZmJzKFDSm8JlPF YhCNHZ8VHR0HTZX6Fr5+TRvmpP0advoxjLz01OWp+oOyHnTcIZ/iMM8/PeBtL0cZ w7yv98t1MiJ2jN2OCkDFnmmsSIEGfqq742vYAtogrRwxJLcjXxx2fAcf7GE2+3CW Zqk7sjLiHn2N8F1if/J/wBeI58MfkRBpdvwRbzUl8Ml1/tLiaqxxdGvUAsZVo+79 NEb+iOekvMadrtuC5fs1+howf8ehpkykqTKGuZSHZfg9Gmj7jNuzsHZrbUjTW7OB pL+9+aAuS7xM/dX3Ykp74H3URc+aty4GBkjqxdNIJD2sSMYxs/lXyx1OmUZWNtT7 NibRDNyCpanb82qz0Dw5 =TsjB -----END PGP SIGNATURE----- --NvtM4XpaobQad1NKvQAmUaKoucx85uD7O--