* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
@ 2017-09-27 17:49 Tadeus Prastowo
2017-09-27 19:31 ` John Wiegley
2017-10-04 18:15 ` Alan Mackenzie
0 siblings, 2 replies; 12+ messages in thread
From: Tadeus Prastowo @ 2017-09-27 17:49 UTC (permalink / raw)
To: 28623
[-- Attachment #1: Type: text/plain, Size: 6148 bytes --]
The following C++14 source code, which is also attached as `mwe.cpp',
shows the indentation problem:
--8<------------------------------
#include <vector>
#include <iostream>
static std::vector<std::vector<unsigned>>
fn(std::vector<std::vector<unsigned>> data) {
return {
{1, 2, 3},
{4, 5, 6},
{7, 8, 9},
};
}
static std::vector<std::vector<unsigned>>
fn(unsigned n, std::vector<std::vector<unsigned>> data) {
return {
{n + 1, n + 2, n + 3},
{n + 4, n + 5, n + 6},
{n + 7, n + 8, n + 9},
};
}
int main() {
/* Expected indentation */
fn({
{1, 2, 3},
{3, 4, 5},
{6, 7, 8},
});
for (const auto &v : fn({
{3, 4, 5},
{6, 7, 8},
{9, 10, 11},
})) {
for (const auto &a : v) {
std::cout << a << '\n';
}
}
/* End: Expected indentation */
/* Problem */
fn(20, {
{1, 2, 3},
{3, 4, 5},
{6, 7, 8},
});
for (const auto &v : fn(20, {
{3, 4, 5},
{6, 7, 8},
{9, 10, 11},
})) {
for (const auto &a : v) {
std::cout << a << '\n';
}
}
/* End: Problem */
}
--8<------------------------------
To fix the problem, I make the following patch:
--8<------------------------------
diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index 05b391a..077e9c9 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -10387,6 +10387,7 @@ comment at the start of cc-engine.el for more info."
(eq (char-after) ?\())
(setq braceassignp 'c++-noassign))
((looking-at c-pre-id-bracelist-key))
+ ((looking-at ",\\s *"))
((looking-at c-return-key))
((and (looking-at c-symbol-start)
(not (looking-at c-keywords-regexp)))
@@ -10398,6 +10399,7 @@ comment at the start of cc-engine.el for more info."
(and (c-go-up-list-backward nil lim) ; FIXME!!! Check
`lim' 2016-07-12.
(eq (char-after) ?\()))
((looking-at c-pre-id-bracelist-key))
+ ((looking-at ",\\s *"))
((looking-at c-return-key))
(t (setq after-type-id-pos (point))
nil))))
--8<------------------------------
Any better suggestion as to how to fix the problem?
Thanks.
In GNU Emacs 27.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.18.9)
of 2017-09-27 built on lgw01-amd64-052
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description: Ubuntu 16.04.3 LTS
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Making completion list...
Quit
Configured using:
'configure --build=x86_64-linux-gnu --prefix=/usr
'--includedir=${prefix}/include' '--mandir=${prefix}/share/man'
'--infodir=${prefix}/share/info' --sysconfdir=/etc --localstatedir=/var
--disable-silent-rules '--libdir=${prefix}/lib/x86_64-linux-gnu'
'--libexecdir=${prefix}/lib/x86_64-linux-gnu' --disable-maintainer-mode
--disable-dependency-tracking --prefix=/usr --sharedstatedir=/var/lib
--program-suffix=-snapshot --with-modules=yes --with-x=yes
--with-x-toolkit=gtk3 --with-xwidgets=yes 'CFLAGS=-g -O2
-fstack-protector-strong -Wformat -Werror=format-security'
'CPPFLAGS=-Wdate-time -D_FORTIFY_SOURCE=2'
'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro''
Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES XWIDGETS LIBSYSTEMD LCMS2
Important settings:
value of $LC_MONETARY: en_US.UTF-8
value of $LC_NUMERIC: en_US.UTF-8
value of $LC_TIME: en_US.UTF-8
value of $LANG: en_US.UTF-8
value of $XMODIFIERS: @im=ibus
locale-coding-system: utf-8-unix
Major mode: C++//l
Minor modes in effect:
tooltip-mode: t
global-eldoc-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
tool-bar-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
line-number-mode: t
transient-mark-mode: t
abbrev-mode: t
Load-path shadows:
None found.
Features:
(shadow sort mail-extr emacsbug message subr-x puny seq byte-opt gv
bytecomp byte-compile cconv dired dired-loaddefs format-spec rfc822 mml
mml-sec password-cache epa derived epg epg-config gnus-util rmail
rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils cc-mode cc-fonts easymenu cc-guess
cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
cl-loaddefs cl-lib elec-pair time-date mule-util tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting xwidget-internal move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)
Memory information:
((conses 16 115959 7786)
(symbols 48 22648 1)
(miscs 40 46 140)
(strings 32 34019 980)
(string-bytes 1 1013691)
(vectors 16 17237)
(vector-slots 8 521799 8627)
(floats 8 50 155)
(intervals 56 397 1)
(buffers 992 13)
(heap 1024 40263 1174))
--
Best regards,
Tadeus
[-- Attachment #2: mwe.cpp --]
[-- Type: text/x-c++src, Size: 1106 bytes --]
#include <vector>
#include <iostream>
static std::vector<std::vector<unsigned>>
fn(std::vector<std::vector<unsigned>> data) {
return {
{1, 2, 3},
{4, 5, 6},
{7, 8, 9},
};
}
static std::vector<std::vector<unsigned>>
fn(unsigned n, std::vector<std::vector<unsigned>> data) {
return {
{n + 1, n + 2, n + 3},
{n + 4, n + 5, n + 6},
{n + 7, n + 8, n + 9},
};
}
int main() {
/* Expected indentation */
fn({
{1, 2, 3},
{3, 4, 5},
{6, 7, 8},
});
for (const auto &v : fn({
{3, 4, 5},
{6, 7, 8},
{9, 10, 11},
})) {
for (const auto &a : v) {
std::cout << a << '\n';
}
}
/* End: Expected indentation */
/* Problem */
fn(20, {
{1, 2, 3},
{3, 4, 5},
{6, 7, 8},
});
for (const auto &v : fn(20, {
{3, 4, 5},
{6, 7, 8},
{9, 10, 11},
})) {
for (const auto &a : v) {
std::cout << a << '\n';
}
}
/* End: Problem */
}
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
2017-09-27 17:49 bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list Tadeus Prastowo
@ 2017-09-27 19:31 ` John Wiegley
2017-10-04 18:15 ` Alan Mackenzie
1 sibling, 0 replies; 12+ messages in thread
From: John Wiegley @ 2017-09-27 19:31 UTC (permalink / raw)
To: Tadeus Prastowo; +Cc: Alan Mackenzie, 28623
>>>>> "TP" == Tadeus Prastowo <tadeus.prastowo@unitn.it> writes:
PT> The following C++14 source code, which is also attached as `mwe.cpp',
PT> shows the indentation problem:
Pinging Alan for this one.
--
John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
2017-09-27 17:49 bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list Tadeus Prastowo
2017-09-27 19:31 ` John Wiegley
@ 2017-10-04 18:15 ` Alan Mackenzie
2017-10-06 2:59 ` Tadeus Prastowo
1 sibling, 1 reply; 12+ messages in thread
From: Alan Mackenzie @ 2017-10-04 18:15 UTC (permalink / raw)
To: Tadeus Prastowo; +Cc: John Wiegley, 28623
Hello, Tadeus.
On Wed, Sep 27, 2017 at 19:49:57 +0200, Tadeus Prastowo wrote:
> The following C++14 source code, which is also attached as `mwe.cpp',
> shows the indentation problem:
> --8<------------------------------
> #include <vector>
> #include <iostream>
> static std::vector<std::vector<unsigned>>
> fn(std::vector<std::vector<unsigned>> data) {
> return {
> {1, 2, 3},
> {4, 5, 6},
> {7, 8, 9},
> };
> }
> static std::vector<std::vector<unsigned>>
> fn(unsigned n, std::vector<std::vector<unsigned>> data) {
> return {
> {n + 1, n + 2, n + 3},
> {n + 4, n + 5, n + 6},
> {n + 7, n + 8, n + 9},
> };
> }
> int main() {
> /* Expected indentation */
> fn({
> {1, 2, 3},
> {3, 4, 5},
> {6, 7, 8},
> });
> for (const auto &v : fn({
> {3, 4, 5},
> {6, 7, 8},
> {9, 10, 11},
> })) {
> for (const auto &a : v) {
> std::cout << a << '\n';
> }
> }
> /* End: Expected indentation */
> /* Problem */
> fn(20, {
> {1, 2, 3},
> {3, 4, 5},
> {6, 7, 8},
> });
> for (const auto &v : fn(20, {
> {3, 4, 5},
> {6, 7, 8},
> {9, 10, 11},
> })) {
> for (const auto &a : v) {
> std::cout << a << '\n';
> }
> }
> /* End: Problem */
> }
> --8<------------------------------
Yes.
> To fix the problem, I make the following patch:
> --8<------------------------------
> diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
> index 05b391a..077e9c9 100644
> --- a/lisp/progmodes/cc-engine.el
> +++ b/lisp/progmodes/cc-engine.el
> @@ -10387,6 +10387,7 @@ comment at the start of cc-engine.el for more info."
> (eq (char-after) ?\())
> (setq braceassignp 'c++-noassign))
> ((looking-at c-pre-id-bracelist-key))
> + ((looking-at ",\\s *"))
> ((looking-at c-return-key))
> ((and (looking-at c-symbol-start)
> (not (looking-at c-keywords-regexp)))
> @@ -10398,6 +10399,7 @@ comment at the start of cc-engine.el for more info."
> (and (c-go-up-list-backward nil lim) ; FIXME!!! Check
> `lim' 2016-07-12.
> (eq (char-after) ?\()))
> ((looking-at c-pre-id-bracelist-key))
> + ((looking-at ",\\s *"))
> ((looking-at c-return-key))
> (t (setq after-type-id-pos (point))
> nil))))
> --8<------------------------------
> Any better suggestion as to how to fix the problem?
Hey, I just love it when people diagnose and fix their own bugs,
particularly in some of the more involved bits of CC Mode. :-)
Just one tiny, tiny, nitpick. in (looking-at ",\\s *"), isn't the "any
amount of space" bit redundant, since we don't use match-end to get the
precise position? In fact, I'm tending towards the simpler (eq
(char-after) ?,).
But, as I say, that's a tiny point in a great piece of debugging. I
will commit this (to the Emacs-26 branch of savannah) soon (from where
it will find its way to the master branch due to some public spirited
person who arranges these things).
Many thanks!
> Thanks.
> In GNU Emacs 27.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.18.9)
> of 2017-09-27 built on lgw01-amd64-052
> Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
> System Description: Ubuntu 16.04.3 LTS
[ .... ]
> Major mode: C++//l
[ .... ]
> --
> Best regards,
> Tadeus
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
2017-10-04 18:15 ` Alan Mackenzie
@ 2017-10-06 2:59 ` Tadeus Prastowo
2017-10-11 20:32 ` Alan Mackenzie
0 siblings, 1 reply; 12+ messages in thread
From: Tadeus Prastowo @ 2017-10-06 2:59 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: John Wiegley, 28623
On Wed, Oct 4, 2017 at 8:15 PM, Alan Mackenzie <acm@muc.de> wrote:
> Hello, Tadeus.
Hi Alan!
[...]
>> Any better suggestion as to how to fix the problem?
>
> Hey, I just love it when people diagnose and fix their own bugs,
> particularly in some of the more involved bits of CC Mode. :-)
To make the maintainer's life easier :-)
> Just one tiny, tiny, nitpick. in (looking-at ",\\s *"), isn't the "any
> amount of space" bit redundant, since we don't use match-end to get the
> precise position? In fact, I'm tending towards the simpler (eq
> (char-after) ?,).
That is surely better. Please go with that solution.
> But, as I say, that's a tiny point in a great piece of debugging. I
> will commit this (to the Emacs-26 branch of savannah) soon (from where
> it will find its way to the master branch due to some public spirited
> person who arranges these things).
Thank you very much for your review.
> Many thanks!
No problem. I'm glad to help.
--
Best regards,
Tadeus
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
2017-10-06 2:59 ` Tadeus Prastowo
@ 2017-10-11 20:32 ` Alan Mackenzie
2017-10-12 11:38 ` Tadeus Prastowo
0 siblings, 1 reply; 12+ messages in thread
From: Alan Mackenzie @ 2017-10-11 20:32 UTC (permalink / raw)
To: Tadeus Prastowo; +Cc: John Wiegley, 28623
Hello, Tadeus.
On Fri, Oct 06, 2017 at 04:59:55 +0200, Tadeus Prastowo wrote:
> On Wed, Oct 4, 2017 at 8:15 PM, Alan Mackenzie <acm@muc.de> wrote:
> Hi Alan!
> [...]
> >> Any better suggestion as to how to fix the problem?
> > Hey, I just love it when people diagnose and fix their own bugs,
> > particularly in some of the more involved bits of CC Mode. :-)
> To make the maintainer's life easier :-)
> > Just one tiny, tiny, nitpick. in (looking-at ",\\s *"), isn't the "any
> > amount of space" bit redundant, since we don't use match-end to get the
> > precise position? In fact, I'm tending towards the simpler (eq
> > (char-after) ?,).
> That is surely better. Please go with that solution.
> > But, as I say, that's a tiny point in a great piece of debugging. I
> > will commit this (to the Emacs-26 branch of savannah) soon (from where
> > it will find its way to the master branch due to some public spirited
> > person who arranges these things).
> Thank you very much for your review.
> > Many thanks!
> No problem. I'm glad to help.
I'm sorry is been a week without any communication from me. The reason
is I've run into problems with other related cases. For example, in
1. auto bad4 = f <3> (
2. {3, 4},
L2 needs to be parsed as an arglist-intro and indented as shown. It's
actually being parsed as a brace-list-open with anchor point on "auto".
What's confusing me is the confusion between a brace list being
recognised by its context (which is what
c-looking-at-or-maybe-in-bracelist mostly does) and by its content. The
{3, 4} above is a brace list by its content but not by its context.
However, it's being wrongly recognised as a by-context brace list, hence
is being parsed and indented wrongly.
I'm not going to have much time to sort this out over the next week or
two, so please bear with me. I haven't forgotten about this.
> --
> Best regards,
> Tadeus
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
2017-10-11 20:32 ` Alan Mackenzie
@ 2017-10-12 11:38 ` Tadeus Prastowo
2017-11-04 19:56 ` Alan Mackenzie
0 siblings, 1 reply; 12+ messages in thread
From: Tadeus Prastowo @ 2017-10-12 11:38 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: John Wiegley, 28623
Hi Alan!
On Wed, Oct 11, 2017 at 10:32 PM, Alan Mackenzie <acm@muc.de> wrote:
> Hello, Tadeus.
[...]
> I'm sorry is been a week without any communication from me. The reason
> is I've run into problems with other related cases. For example, in
>
> 1. auto bad4 = f <3> (
> 2. {3, 4},
>
> L2 needs to be parsed as an arglist-intro and indented as shown. It's
> actually being parsed as a brace-list-open with anchor point on "auto".
>
> What's confusing me is the confusion between a brace list being
> recognised by its context (which is what
> c-looking-at-or-maybe-in-bracelist mostly does) and by its content. The
> {3, 4} above is a brace list by its content but not by its context.
> However, it's being wrongly recognised as a by-context brace list, hence
> is being parsed and indented wrongly.
>
> I'm not going to have much time to sort this out over the next week or
> two, so please bear with me. I haven't forgotten about this.
Thanks for sharing the problem with me. I will also look into the
matter during this weekend. Hopefully I can come up with a good
solution :)
> --
> Alan Mackenzie (Nuremberg, Germany).
--
Best regards,
Tadeus
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
2017-10-12 11:38 ` Tadeus Prastowo
@ 2017-11-04 19:56 ` Alan Mackenzie
2017-11-06 22:46 ` Tadeus Prastowo
0 siblings, 1 reply; 12+ messages in thread
From: Alan Mackenzie @ 2017-11-04 19:56 UTC (permalink / raw)
To: Tadeus Prastowo; +Cc: John Wiegley, 28623
Hello, Tadeus.
On Thu, Oct 12, 2017 at 13:38:56 +0200, Tadeus Prastowo wrote:
> Hi Alan!
> On Wed, Oct 11, 2017 at 10:32 PM, Alan Mackenzie <acm@muc.de> wrote:
> > Hello, Tadeus.
> [...]
> > I'm sorry is been a week without any communication from me. The reason
> > is I've run into problems with other related cases. For example, in
> > 1. auto bad4 = f <3> (
> > 2. {3, 4},
> > L2 needs to be parsed as an arglist-intro and indented as shown. It's
> > actually being parsed as a brace-list-open with anchor point on "auto".
> > What's confusing me is the confusion between a brace list being
> > recognised by its context (which is what
> > c-looking-at-or-maybe-in-bracelist mostly does) and by its content. The
> > {3, 4} above is a brace list by its content but not by its context.
> > However, it's being wrongly recognised as a by-context brace list, hence
> > is being parsed and indented wrongly.
> > I'm not going to have much time to sort this out over the next week or
> > two, so please bear with me. I haven't forgotten about this.
> Thanks for sharing the problem with me. I will also look into the
> matter during this weekend. Hopefully I can come up with a good
> solution :)
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.
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.
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.
Here's the patch: have fun!
diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index 6f39cc6433..2da9e98345 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -10426,17 +10426,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 +10448,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 +10491,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 +10549,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 +10574,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,7 +10598,7 @@ 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 by-content)
;; 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
@@ -10632,14 +10637,22 @@ 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 lim))
+ t)
+ (setq containing-sexp next-containing
+ lim nil
+ next-containing nil)))))
+ (cond
+ ((eq bufpos t)
+ (and containing-sexp
+ (not (c-looking-at-statement-block))))
+ ((consp bufpos)
+ (and (or by-content (not (eq (cdr bufpos) 'in-paren)))
+ (car bufpos)))
+ (t nil))))))
(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 +11519,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 +11682,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 +11760,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 +11786,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 +11830,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 +12253,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 +12414,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 +12497,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 +12538,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 +12586,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 +12612,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 +12634,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)
@@ -12750,10 +12765,10 @@ c-guess-basic-syntax
(if (>= (point) placeholder)
(progn
(forward-char)
- (skip-chars-forward " \t"))
+ (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 +12842,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).
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
2017-11-04 19:56 ` Alan Mackenzie
@ 2017-11-06 22:46 ` Tadeus Prastowo
2017-11-08 19:23 ` Alan Mackenzie
[not found] ` <20171108192358.GA4582@ACM>
0 siblings, 2 replies; 12+ messages in thread
From: Tadeus Prastowo @ 2017-11-06 22:46 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: John Wiegley, 28623
Hi Alan!
On Sat, Nov 4, 2017 at 8:56 PM, Alan Mackenzie <acm@muc.de> wrote:
> Hello, Tadeus.
>
> On Thu, Oct 12, 2017 at 13:38:56 +0200, Tadeus Prastowo wrote:
>> Hi Alan!
>
>> On Wed, Oct 11, 2017 at 10:32 PM, Alan Mackenzie <acm@muc.de> wrote:
>> > Hello, Tadeus.
>
>> [...]
>
>> > I'm sorry is been a week without any communication from me. The reason
>> > is I've run into problems with other related cases. For example, in
>
>> > 1. auto bad4 = f <3> (
>> > 2. {3, 4},
>
>> > L2 needs to be parsed as an arglist-intro and indented as shown. It's
>> > actually being parsed as a brace-list-open with anchor point on "auto".
>
>> > What's confusing me is the confusion between a brace list being
>> > recognised by its context (which is what
>> > c-looking-at-or-maybe-in-bracelist mostly does) and by its content. The
>> > {3, 4} above is a brace list by its content but not by its context.
>> > However, it's being wrongly recognised as a by-context brace list, hence
>> > is being parsed and indented wrongly.
>
>> > I'm not going to have much time to sort this out over the next week or
>> > two, so please bear with me. I haven't forgotten about this.
>
>> Thanks for sharing the problem with me. I will also look into the
>> matter during this weekend. Hopefully I can come up with a good
>> solution :)
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:
-- 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.
> 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.
[...]
> --
> Alan Mackenzie (Nuremberg, Germany).
--
Best regards,
Tadeus
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
2017-11-06 22:46 ` Tadeus Prastowo
@ 2017-11-08 19:23 ` Alan Mackenzie
[not found] ` <20171108192358.GA4582@ACM>
1 sibling, 0 replies; 12+ messages in thread
From: Alan Mackenzie @ 2017-11-08 19:23 UTC (permalink / raw)
To: Tadeus Prastowo; +Cc: John Wiegley, 28623
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 <acm@muc.de> 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).
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
[not found] ` <20171108192358.GA4582@ACM>
@ 2017-11-09 9:27 ` Tadeus Prastowo
2017-11-09 18:53 ` Alan Mackenzie
[not found] ` <20171109185354.GA15085@ACM>
0 siblings, 2 replies; 12+ messages in thread
From: Tadeus Prastowo @ 2017-11-09 9:27 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: John Wiegley, 28623
On Wed, Nov 8, 2017 at 8:23 PM, Alan Mackenzie <acm@muc.de> wrote:
[...]
> 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.
My pleasure. And thank you very much for looking into this last
problem as well.
> 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.
Cool! Your new patch produces the following now:
int main() {
/* Indentation produced by your new 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: Indentation produced by your new patch */
}
So, you solved the problem :)
>> 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.
I will have a look at it this weekend.
>> > 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.
You are welcome.
And, just out of curiosity, in cc-engine, there is a long function
with many inline comments in the form of CASE xxx. Why aren't those
refactored into individual functions? Performance issue?
> --
> Alan Mackenzie (Nuremberg, Germany).
--
Best regards,
Tadeus
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
2017-11-09 9:27 ` Tadeus Prastowo
@ 2017-11-09 18:53 ` Alan Mackenzie
[not found] ` <20171109185354.GA15085@ACM>
1 sibling, 0 replies; 12+ messages in thread
From: Alan Mackenzie @ 2017-11-09 18:53 UTC (permalink / raw)
To: Tadeus Prastowo; +Cc: John Wiegley, 28623-done
Hello, Tadeus.
On Thu, Nov 09, 2017 at 10:27:55 +0100, Tadeus Prastowo wrote:
> On Wed, Nov 8, 2017 at 8:23 PM, Alan Mackenzie <acm@muc.de> wrote:
[...]
> > 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.
> My pleasure. And thank you very much for looking into this last
> problem as well.
I've committed the patch to the canonical places, including the emacs-26
branch at savannah (whence it will find it's way into master), and I'm
closing the bug.
[ .... ]
> And, just out of curiosity, in cc-engine, there is a long function
> with many inline comments in the form of CASE xxx. Why aren't those
> refactored into individual functions? Performance issue?
There are two such functions, c-forward-decl-or-cast-1 and
c-guess-basic-syntax. Both of them have LOTS of local variables which
would have to be passed into smaller individual functions, and sometimes
those functions would have to alter the "more global" version of the
variable.
Doing this would indeed be slower, but probably not very much. I suspect
all the parameter passing would be awkward. But it's worth stating that
my predecessor, Martin Stjernholm, extracted c-guess-continued-construct
from c-guess-basic-syntax, which shows that it is possible.
> --
> Best regards,
> Tadeus
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list
[not found] ` <20171109185354.GA15085@ACM>
@ 2017-11-10 12:07 ` Tadeus Prastowo
0 siblings, 0 replies; 12+ messages in thread
From: Tadeus Prastowo @ 2017-11-10 12:07 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: John Wiegley, 28623-done
Hi Alan!
On Thu, Nov 9, 2017 at 7:53 PM, Alan Mackenzie <acm@muc.de> wrote:
> Hello, Tadeus.
>
> On Thu, Nov 09, 2017 at 10:27:55 +0100, Tadeus Prastowo wrote:
[...]
> I've committed the patch to the canonical places, including the emacs-26
> branch at savannah (whence it will find it's way into master), and I'm
> closing the bug.
Thank you very much.
>> And, just out of curiosity, in cc-engine, there is a long function
>> with many inline comments in the form of CASE xxx. Why aren't those
>> refactored into individual functions? Performance issue?
>
> There are two such functions, c-forward-decl-or-cast-1 and
> c-guess-basic-syntax. Both of them have LOTS of local variables which
> would have to be passed into smaller individual functions, and sometimes
> those functions would have to alter the "more global" version of the
> variable.
>
> Doing this would indeed be slower, but probably not very much. I suspect
> all the parameter passing would be awkward. But it's worth stating that
> my predecessor, Martin Stjernholm, extracted c-guess-continued-construct
> from c-guess-basic-syntax, which shows that it is possible.
I see. Thank you very much for kindly explaining that to me.
> --
> Alan Mackenzie (Nuremberg, Germany).
--
Best regards,
Tadeus
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-11-10 12:07 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-27 17:49 bug#28623: 27.0.50; lisp/progmodes/cc-engine.el incorrect indentation of C++14 curly-brace initializer list Tadeus Prastowo
2017-09-27 19:31 ` John Wiegley
2017-10-04 18:15 ` Alan Mackenzie
2017-10-06 2:59 ` Tadeus Prastowo
2017-10-11 20:32 ` Alan Mackenzie
2017-10-12 11:38 ` Tadeus Prastowo
2017-11-04 19:56 ` Alan Mackenzie
2017-11-06 22:46 ` Tadeus Prastowo
2017-11-08 19:23 ` Alan Mackenzie
[not found] ` <20171108192358.GA4582@ACM>
2017-11-09 9:27 ` Tadeus Prastowo
2017-11-09 18:53 ` Alan Mackenzie
[not found] ` <20171109185354.GA15085@ACM>
2017-11-10 12:07 ` Tadeus Prastowo
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).