From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.bugs Subject: bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list Date: Wed, 8 Nov 2017 19:23:58 +0000 Message-ID: <20171108192358.GA4582__22566.0242060714$1510169210$gmane$org@ACM> References: <20171004181552.GA15562@ACM> <20171011203221.GB7384@ACM> <20171104195639.GA5384@ACM> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1510169210 25505 195.159.176.226 (8 Nov 2017 19:26:50 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 8 Nov 2017 19:26:50 +0000 (UTC) User-Agent: Mutt/1.7.2 (2016-11-26) Cc: John Wiegley , 28623@debbugs.gnu.org To: Tadeus Prastowo Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Nov 08 20:26:45 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eCVzn-0006OP-Lf for geb-bug-gnu-emacs@m.gmane.org; Wed, 08 Nov 2017 20:26:44 +0100 Original-Received: from localhost ([::1]:33620 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eCVzu-0002vH-V8 for geb-bug-gnu-emacs@m.gmane.org; Wed, 08 Nov 2017 14:26:50 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:42241) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eCVzI-0002bT-Dr for bug-gnu-emacs@gnu.org; Wed, 08 Nov 2017 14:26:14 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eCVzG-00032o-3H for bug-gnu-emacs@gnu.org; Wed, 08 Nov 2017 14:26:12 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:50222) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eCVz8-00030s-8P; Wed, 08 Nov 2017 14:26:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1eCVz8-0001Er-20; Wed, 08 Nov 2017 14:26:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org, bug-cc-mode@gnu.org Resent-Date: Wed, 08 Nov 2017 19:26:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 28623 X-GNU-PR-Package: emacs,cc-mode X-GNU-PR-Keywords: Original-Received: via spool by 28623-submit@debbugs.gnu.org id=B28623.15101691034685 (code B ref 28623); Wed, 08 Nov 2017 19:26:02 +0000 Original-Received: (at 28623) by debbugs.gnu.org; 8 Nov 2017 19:25:03 +0000 Original-Received: from localhost ([127.0.0.1]:58903 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eCVyA-0001DV-Qz for submit@debbugs.gnu.org; Wed, 08 Nov 2017 14:25:03 -0500 Original-Received: from ocolin.muc.de ([193.149.48.4]:47213 helo=mail.muc.de) by debbugs.gnu.org with smtp (Exim 4.84_2) (envelope-from ) id 1eCVy8-0001Cq-1I for 28623@debbugs.gnu.org; Wed, 08 Nov 2017 14:25:00 -0500 Original-Received: (qmail 21678 invoked by uid 3782); 8 Nov 2017 19:24:56 -0000 Original-Received: from acm.muc.de (p548C6D8A.dip0.t-ipconnect.de [84.140.109.138]) by colin.muc.de (tmda-ofmipd) with ESMTP; Wed, 08 Nov 2017 20:24:55 +0100 Original-Received: (qmail 5248 invoked by uid 1000); 8 Nov 2017 19:23:58 -0000 Content-Disposition: inline In-Reply-To: X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de 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" Xref: news.gmane.org gmane.emacs.bugs:139623 Archived-At: Hello again, Tadeus. On Mon, Nov 06, 2017 at 23:46:26 +0100, Tadeus Prastowo wrote: > Hi Alan! > On Sat, Nov 4, 2017 at 8:56 PM, Alan Mackenzie wrote: > > Hello, Tadeus. [ .... ] > Sorry that I did not manage to spare the last three weekends to look > into the problem :( > > I think I've solved this, though it's been perhaps the most difficult bug > > in CC Mode for some years. Each time I thought I'd nailed it, some > > awkward test case would misbehave. > Thank you very much for working on it. > > Anyhow, would you please try the patch below, which should apply cleanly > > to either the emacs-26 branch or master. It is not finished; for example > > I've still got to amend several comments. Nevertheless I think it's > > working. I look forward to hearing of any problems which are still in > > this patch, or of a report that it seems to be working. > The patch applied to master cleanly. > Against the mwe.cpp I sent the other day, the patch works fine. But, > it does not work with the following one: OK. The essential characteristic of your new file is: ({ {..}, ..., {...}}, { {..} ..... ^ l L With the critical point marked, c-inside-bracelist-p had calculated a backward search limit at position L, which was insufficient for it to determine its brace list characteristic. I've corrected c-inside-bracelist-p such that it now uses position l as this limit. I've also taken the opportunity to simplify it quite a bit. This now appears to work. So, thank you for taking the time to test this, and finding this further bug. Could I ask you, please, to try the amended patch which I include below. This should, again, apply cleanly to the emacs-26 branch, or master. It is a patch "from scratch"; it is not an incremental patch on top of the last one. > -- 8< ---------------------------------- > int main() { > /* Indentation produced by my patch the other day */ > fn({ > {1, 2, 3}, > {3, 4, 5}, > {6, 7, 8}, > }, { > {1, 3}, > {4, 5}, > {7, 8}, > }); > for (const auto &v : fn({ > {3, 4, 5}, > {6, 7, 8}, > {9, 10, 11}, > }, { > {1, 3}, > {4, 5}, > {7, 8}, > })) { > for (const auto &a : v) { > std::cout << a << '\n'; > } > } > /* End: Indentation produced by my patch the other day */ > /* Problem observed using your patch */ > fn({ > {1, 2, 3}, > {3, 4, 5}, > {6, 7, 8}, > }, { > {1, 3}, > {4, 5}, > {7, 8}, > }); > for (const auto &v : fn({ > {3, 4, 5}, > {6, 7, 8}, > {9, 10, 11}, > }, { > {1, 3}, > {4, 5}, > {7, 8}, > })) { > for (const auto &a : v) { > std::cout << a << '\n'; > } > } > /* End: Problem observed using your patch */ > } > -- 8< ---------------------------------- > Additionally, I would argue that compared to the one produced by my > patch demonstrated above, the following indentation would be even > better: > for (const auto &v : fn({ > {3, 4, 5}, > {6, 7, 8}, > {9, 10, 11}, > }, { > {1, 3}, > {4, 5}, > {7, 8}, > })) { > for (const auto &a : v) { > std::cout << a << '\n'; > } > } > Please let me know what you think about that. I think I would agree with you. This is probably fixable by configuring the CC Mode indentation engine, possibly by writing a Line-up function, but I can't say for sure without looking at it more closely. > > If you have nothing against it, I intend to put your test file (or bits > > of it) into a new file in the CC Mode test suite. > Yes, that is okay. Thanks, I'll do that. > [...] Here's the amended patch: diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el index 6f39cc6433..982be3bb3b 100644 --- a/lisp/progmodes/cc-engine.el +++ b/lisp/progmodes/cc-engine.el @@ -10407,16 +10407,20 @@ c-backward-over-enum-header (defun c-looking-at-or-maybe-in-bracelist (&optional containing-sexp lim) ;; Point is at an open brace. If this starts a brace list, return a list ;; whose car is the buffer position of the start of the construct which - ;; introduces the list, and whose cdr is t if we have parsed a keyword - ;; matching `c-opt-inexpr-brace-list-key' (e.g. Java's "new"), nil - ;; otherwise. Otherwise, if point might be inside an enclosing brace list, - ;; return t. If point is definitely neither at nor in a brace list, return - ;; nil. + ;; introduces the list, and whose cdr is the symbol `in-paren' if the brace + ;; is directly enclosed in a parenthesis form (i.e. an arglist), t if we + ;; have parsed a keyword matching `c-opt-inexpr-brace-list-key' (e.g. Java's + ;; "new"), nil otherwise. Otherwise, if point might be inside an enclosing + ;; brace list, return t. If point is definitely neither at nor in a brace + ;; list, return nil. ;; ;; CONTAINING-SEXP is the position of the brace/paren/bracket enclosing ;; POINT, or nil if there is no such position, or we do not know it. LIM is ;; a backward search limit. ;; + ;; The determination of whether the brace starts a brace list is solely by + ;; the context of the brace, not by its contents. + ;; ;; Here, "brace list" does not include the body of an enum. (save-excursion (let ((start (point)) @@ -10426,17 +10430,20 @@ c-looking-at-or-maybe-in-bracelist (and (c-major-mode-is 'pike-mode) c-decl-block-key)) (braceassignp 'dontknow) - inexpr-brace-list bufpos macro-start res pos after-type-id-pos) + inexpr-brace-list bufpos macro-start res pos after-type-id-pos + in-paren) (setq res (c-backward-token-2 1 t lim)) ;; Checks to do only on the first sexp before the brace. ;; Have we a C++ initialization, without an "="? (if (and (c-major-mode-is 'c++-mode) (cond - ((and (not (eq res 0)) + ((and (or (not (eq res 0)) + (eq (char-after) ?,)) (c-go-up-list-backward nil lim) ; FIXME!!! Check ; `lim' 2016-07-12. (eq (char-after) ?\()) - (setq braceassignp 'c++-noassign)) + (setq braceassignp 'c++-noassign + in-paren 'in-paren)) ((looking-at c-pre-id-bracelist-key)) ((looking-at c-return-key)) ((and (looking-at c-symbol-start) @@ -10445,9 +10452,11 @@ c-looking-at-or-maybe-in-bracelist (t nil)) (save-excursion (cond - ((not (eq res 0)) + ((or (not (eq res 0)) + (eq (char-after) ?,)) (and (c-go-up-list-backward nil lim) ; FIXME!!! Check `lim' 2016-07-12. - (eq (char-after) ?\())) + (eq (char-after) ?\() + (setq in-paren 'in-paren))) ((looking-at c-pre-id-bracelist-key)) ((looking-at c-return-key)) (t (setq after-type-id-pos (point)) @@ -10486,7 +10495,7 @@ c-looking-at-or-maybe-in-bracelist (c-backward-syntactic-ws) (eq (char-before) ?\())) ;; Single identifier between '(' and '{'. We have a bracelist. - (cons after-type-id-pos nil)) + (cons after-type-id-pos 'in-paren)) (t (goto-char pos) @@ -10544,7 +10553,7 @@ c-looking-at-or-maybe-in-bracelist (braceassignp ;; We've hit the beginning of the aggregate list. (c-beginning-of-statement-1 containing-sexp) - (cons (point) inexpr-brace-list)) + (cons (point) (or in-paren inexpr-brace-list))) ((and after-type-id-pos (save-excursion (when (eq (char-after) ?\;) @@ -10569,7 +10578,7 @@ c-looking-at-or-maybe-in-bracelist nil nil)) (and (consp res) (eq (car res) after-type-id-pos)))))) - (cons bufpos inexpr-brace-list)) + (cons bufpos (or in-paren inexpr-brace-list))) ((eq (char-after) ?\;) ;; Brace lists can't contain a semicolon, so we're done. ;; (setq containing-sexp nil) @@ -10593,12 +10602,16 @@ c-looking-at-or-maybe-in-bracelist (t t)))) ;; The caller can go up one level. ))) -(defun c-inside-bracelist-p (containing-sexp paren-state) +(defun c-inside-bracelist-p (containing-sexp paren-state accept-in-paren) ;; return the buffer position of the beginning of the brace list ;; statement if we're inside a brace list, otherwise return nil. ;; CONTAINING-SEXP is the buffer pos of the innermost containing ;; paren. PAREN-STATE is the remainder of the state of enclosing - ;; braces + ;; braces. ACCEPT-IN-PAREN is non-nil iff we will accept as a brace + ;; list a brace directly enclosed in a parenthesis. + ;; + ;; The "brace list" here is recognized solely by its context, not by + ;; its contents. ;; ;; N.B.: This algorithm can potentially get confused by cpp macros ;; placed in inconvenient locations. It's a trade-off we make for @@ -10613,17 +10626,11 @@ c-inside-bracelist-p ;; this will pick up array/aggregate init lists, even if they are nested. (save-excursion (let ((bufpos t) - lim next-containing) + next-containing) (while (and (eq bufpos t) containing-sexp) (when paren-state - (if (consp (car paren-state)) - (setq lim (cdr (car paren-state)) - paren-state (cdr paren-state)) - (setq lim (car paren-state))) - (when paren-state - (setq next-containing (car paren-state) - paren-state (cdr paren-state)))) + (setq next-containing (c-pull-open-brace paren-state))) (goto-char containing-sexp) (if (c-looking-at-inexpr-block next-containing next-containing) @@ -10632,14 +10639,16 @@ c-inside-bracelist-p ;; containing sexp, so that c-looking-at-inexpr-block ;; doesn't check for an identifier before it. (setq bufpos nil) - (when (or (not (eq (char-after) ?{)) - (eq (setq bufpos (c-looking-at-or-maybe-in-bracelist - next-containing lim)) - t)) - (setq containing-sexp next-containing - lim nil - next-containing nil)))) - (and (consp bufpos) (car bufpos)))))) + (if (not (eq (char-after) ?{)) + (setq bufpos nil) + (when (eq (setq bufpos (c-looking-at-or-maybe-in-bracelist + next-containing next-containing)) + t) + (setq containing-sexp next-containing + next-containing nil))))) + (and (consp bufpos) + (or accept-in-paren (not (eq (cdr bufpos) 'in-paren))) + (car bufpos)))))) (defun c-looking-at-special-brace-list (&optional _lim) ;; If we're looking at the start of a pike-style list, i.e., `({ })', @@ -11506,6 +11515,7 @@ c-guess-basic-syntax ;; The paren state outside `containing-sexp', or at ;; `indent-point' if `containing-sexp' is nil. (paren-state (c-parse-state)) + (state-cache (copy-tree paren-state)) ;; There's always at most one syntactic element which got ;; an anchor pos. It's stored in syntactic-relpos. syntactic-relpos @@ -11668,7 +11678,7 @@ c-guess-basic-syntax (not (c-at-vsemi-p before-ws-ip)) (not (memq char-after-ip '(?\) ?\] ?,))) (or (not (eq char-before-ip ?})) - (c-looking-at-inexpr-block-backward c-state-cache)) + (c-looking-at-inexpr-block-backward state-cache)) (> (point) (progn ;; Ought to cache the result from the @@ -11746,7 +11756,7 @@ c-guess-basic-syntax (if containing-sexp (progn (goto-char containing-sexp) - (setq lim (c-most-enclosing-brace c-state-cache + (setq lim (c-most-enclosing-brace state-cache containing-sexp)) (c-backward-to-block-anchor lim) (c-add-stmt-syntax 'case-label nil t lim paren-state)) @@ -11772,7 +11782,7 @@ c-guess-basic-syntax (containing-sexp (goto-char containing-sexp) - (setq lim (c-most-enclosing-brace c-state-cache + (setq lim (c-most-enclosing-brace state-cache containing-sexp)) (save-excursion (setq tmpsymbol @@ -11816,7 +11826,7 @@ c-guess-basic-syntax (goto-char (cdr placeholder)) (back-to-indentation) (c-add-stmt-syntax tmpsymbol nil t - (c-most-enclosing-brace c-state-cache (point)) + (c-most-enclosing-brace state-cache (point)) paren-state) (unless (eq (point) (cdr placeholder)) (c-add-syntax (car placeholder)))) @@ -12239,11 +12249,11 @@ c-guess-basic-syntax (and (eq (char-before) ?}) (save-excursion (let ((start (point))) - (if (and c-state-cache - (consp (car c-state-cache)) - (eq (cdar c-state-cache) (point))) + (if (and state-cache + (consp (car state-cache)) + (eq (cdar state-cache) (point))) ;; Speed up the backward search a bit. - (goto-char (caar c-state-cache))) + (goto-char (caar state-cache))) (c-beginning-of-decl-1 containing-sexp) ; Can't use `lim' here. (setq placeholder (point)) (if (= start (point)) @@ -12400,7 +12410,8 @@ c-guess-basic-syntax ((and (eq char-after-ip ?{) (progn (setq placeholder (c-inside-bracelist-p (point) - paren-state)) + paren-state + nil)) (if placeholder (setq tmpsymbol '(brace-list-open . inexpr-class)) (setq tmpsymbol '(block-open . inexpr-statement) @@ -12482,7 +12493,7 @@ c-guess-basic-syntax (skip-chars-forward " \t")) (goto-char placeholder)) (c-add-stmt-syntax 'arglist-cont-nonempty (list containing-sexp) t - (c-most-enclosing-brace c-state-cache (point)) + (c-most-enclosing-brace state-cache (point)) paren-state)) ;; CASE 7G: we are looking at just a normal arglist @@ -12523,7 +12534,7 @@ c-guess-basic-syntax (save-excursion (goto-char containing-sexp) (c-looking-at-special-brace-list))) - (c-inside-bracelist-p containing-sexp paren-state)))) + (c-inside-bracelist-p containing-sexp paren-state t)))) (cond ;; CASE 9A: In the middle of a special brace list opener. @@ -12571,7 +12582,7 @@ c-guess-basic-syntax (= (point) containing-sexp))) (if (eq (point) (c-point 'boi)) (c-add-syntax 'brace-list-close (point)) - (setq lim (c-most-enclosing-brace c-state-cache (point))) + (setq lim (c-most-enclosing-brace state-cache (point))) (c-beginning-of-statement-1 lim nil nil t) (c-add-stmt-syntax 'brace-list-close nil t lim paren-state))) @@ -12597,7 +12608,7 @@ c-guess-basic-syntax (goto-char containing-sexp)) (if (eq (point) (c-point 'boi)) (c-add-syntax 'brace-list-intro (point)) - (setq lim (c-most-enclosing-brace c-state-cache (point))) + (setq lim (c-most-enclosing-brace state-cache (point))) (c-beginning-of-statement-1 lim) (c-add-stmt-syntax 'brace-list-intro nil t lim paren-state))) @@ -12619,7 +12630,7 @@ c-guess-basic-syntax ((and (not (memq char-before-ip '(?\; ?:))) (not (c-at-vsemi-p before-ws-ip)) (or (not (eq char-before-ip ?})) - (c-looking-at-inexpr-block-backward c-state-cache)) + (c-looking-at-inexpr-block-backward state-cache)) (> (point) (save-excursion (c-beginning-of-statement-1 containing-sexp) @@ -12753,7 +12764,7 @@ c-guess-basic-syntax (skip-chars-forward " \t")) (goto-char placeholder)) (c-add-stmt-syntax 'template-args-cont (list containing-<) t - (c-most-enclosing-brace c-state-cache (point)) + (c-most-enclosing-brace state-cache (point)) paren-state)) ;; CASE 17: Statement or defun catchall. @@ -12827,7 +12838,7 @@ c-guess-basic-syntax (goto-char (cdr placeholder)) (back-to-indentation) (c-add-stmt-syntax tmpsymbol nil t - (c-most-enclosing-brace c-state-cache (point)) + (c-most-enclosing-brace state-cache (point)) paren-state) (if (/= (point) (cdr placeholder)) (c-add-syntax (car placeholder)))) > -- > Best regards, > Tadeus -- Alan Mackenzie (Nuremberg, Germany).