bug#24431: 25.1.50; Wrong indentation in C++ mode when calling constructors with braces

bug#24431: 25.1.50; Wrong indentation in C++ mode when calling constructors with braces

[Matthias Meulien]

Just type the following lines in a buffer with c++-mode enabled:

struct line_t;
struct point_t;

void set_line(line_t);

set_line(line_t(point_t(0.4, 0.2),
		point_t(0.2, 0.5)));

set_line(line_t{point_t{0.4, 0.2},
      point_t{0.2, 0.5}}); // wrong identation




In GNU Emacs 25.1.50.2 (x86_64-unknown-linux-gnu, GTK+ Version 3.20.9)
 of 2016-08-27 built on carbon
Repository revision: 1c7166a3ba399efbefc65feb8a6a41ef0dfa33a4
Windowing system distributor 'The X.Org Foundation', version
11.0.11804000
System Description:	Arch Linux

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark set [2 times]
next-line: End of buffer [2 times]

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 LIBSYSTEMD

Important settings:
  value of $LANG: fr_FR.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
  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 cl-macs subr-x puny seq byte-
opt
bytecomp byte-compile cl-extra help-mode 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 gv cl-loaddefs pcase cl-lib 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 newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow 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 charscript 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 dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 119083 7504)
 (symbols 48 23053 0)
 (miscs 40 110 129)
 (strings 32 23513 4635)
 (string-bytes 1 842378)
 (vectors 16 16890)
 (vector-slots 8 475357 2393)
 (floats 8 192 56)
 (intervals 56 535 172)
 (buffers 976 12))

bug#24431: 25.1.50; Wrong indentation in C++ mode when calling constructors with braces

[Alan Mackenzie]

Hello, Matthias

In article <mailman.2380.1473801128.22741.bug-gnu-emacs@gnu.org> you wrote:

> Just type the following lines in a buffer with c++-mode enabled:

> struct line_t;
> struct point_t;

> void set_line(line_t);

> set_line(line_t(point_t(0.4, 0.2),
>                 point_t(0.2, 0.5)));

> set_line(line_t{point_t{0.4, 0.2},
>       point_t{0.2, 0.5}}); // wrong identation

What, exactly, is wrong about that indentation?  What would you like to
see there, and why?

When I do C-c C-s on that last line, I get:

    Syntactic analysis: ((arglist-cont-nonempty 130 138) (defun-block-intro 130) (statement-cont 130))

, which seems to be incorrect, since neither of the anchor points in the
last two of the three elements point at locations within the internal C++
constructs.  Amongst other things.

> In GNU Emacs 25.1.50.2 (x86_64-unknown-linux-gnu, GTK+ Version 3.20.9)
>  of 2016-08-27 built on carbon
> Repository revision: 1c7166a3ba399efbefc65feb8a6a41ef0dfa33a4
> Windowing system distributor 'The X.Org Foundation', version
> 11.0.11804000
> System Description:     Arch Linux

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#24431: 25.1.50; Wrong indentation in C++ mode when calling constructors with braces

[Matthias Meulien]

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

Hi Alan,

Sorry, my report lacks of precision.

> > set_line(line_t(point_t(0.4, 0.2),
> >                 point_t(0.2, 0.5)));
> 
> > 
> > set_line(line_t{point_t{0.4, 0.2},
> >       point_t{0.2, 0.5}}); // wrong identation
> 
> > What, exactly, is wrong about that indentation?  What would you like
to see there, and why?

I am expecting to see the same indentation whatever I'm using, brackets
or parenthesis; That is:

set_line(line_t{point_t{0.4, 0.2},
                point_t{0.2, 0.5}});
> 
-- 
Matthias

[-- Attachment #2: Type: text/html, Size: 1075 bytes --]

bug#24431: 25.1.50; Wrong indentation in C++ mode when calling constructors with braces

[Alan Mackenzie]

Hello, Matthias.

Happy New Year!

On Thu, Sep 15, 2016 at 02:09:24PM +0200, Matthias Meulien wrote:
> Hi Alan,

> Sorry, my report lacks of precision.

> > > set_line(line_t(point_t(0.4, 0.2),
> > >                 point_t(0.2, 0.5)));


> > > set_line(line_t{point_t{0.4, 0.2},
> > >       point_t{0.2, 0.5}}); // wrong identation

> > > What, exactly, is wrong about that indentation?  What would you like
> to see there, and why?

> I am expecting to see the same indentation whatever I'm using, brackets
> or parenthesis; That is:

> set_line(line_t{point_t{0.4, 0.2},
>                 point_t{0.2, 0.5}});

This bug proved quite challenging to fix, and I'm still not finished
with it.  The syntactic analysis of the construct, at the deepest level
of CC Mode, was faulty, and I had to add a new facility to the
indentation engine for "an absolute column which won't be amended by
further relative offsets" (but don't worry about that).

With the patch below, the troublesome line gets indented how we would
both want it.  Please note that at the moment, this only works in the CC
Mode style "gnu", but you've got that set in your file local variables
anyway.

After applying the patch and recompiling CC Mode, if you are in a
running Emacs, please do "M-: (makunbound 'c-style-alist)" and then M-x
load-file <CR> .../lisp/progmodes/cc-styles.elc <CR>.  This will cause
the variable c-style-alist to be updated to its new value, which simple
loading of the file wouldn't achieve.

What doesn't work so well is that if that construct is expanded onto
several lines (with each brace on its own line) it indents like this:

    set_line(line_t
             {
               point_t
                 {
                   0.4, 0.2
                     },
                 point_t
                   {
                     0.2, 0.5
                       }
             }
             );

, and also the syntactic analysis shows 'statement-block-intro where it
should really be 'brace-list-intro, and so on.  I'll be working to fix
this.

Anyhow, here's the patch.  Please try it out, and let me know how well
(or how badly) it works.  As I said, I'm still working to fix some
things.


diff -r ea743536b883 cc-align.el
--- a/cc-align.el	Fri Dec 30 15:19:21 2016 +0000
+++ b/cc-align.el	Sun Jan 01 14:17:56 2017 +0000
@@ -278,7 +278,7 @@
     (beginning-of-line)
     (backward-up-list 1)
     (skip-chars-forward " \t" (c-point 'eol))
-    (vector (1+ (current-column)))))
+    (vector (list (1+ (current-column))))))
 
 (defun c-lineup-arglist-close-under-paren (langelem)
   "Line up a line under the enclosing open paren.
diff -r ea743536b883 cc-engine.el
--- a/cc-engine.el	Fri Dec 30 15:19:21 2016 +0000
+++ b/cc-engine.el	Sun Jan 01 14:17:56 2017 +0000
@@ -10382,6 +10382,30 @@
   (c-at-statement-start-p))
 (make-obsolete 'c-looking-at-bos 'c-at-statement-start-p nil)
 
+(defun c-looking-at-statement-block ()
+  ;; Point is at an opening brace.  If this is a statement block (i.e. the
+  ;; elements in it are terminated by semicolons) return t.  Otherwise, return
+  ;; nil.
+  (let ((here (point)))
+    (prog1
+	(if (c-go-list-forward)
+	    (let ((there (point)))
+	      (backward-char)
+	      (c-syntactic-skip-backward
+	       "^;," here t)
+	      (cond
+	       ((eq (char-before) ?\;) t)
+	       ((eq (char-before) ?,) nil)
+	       (t (goto-char here)
+		  (forward-char)
+		  (and (c-syntactic-re-search-forward "{" there t t)
+		       (progn (backward-char)
+			      (c-looking-at-statement-block))))))
+	  (forward-char)
+	  (and (c-syntactic-re-search-forward "[;,]" nil t t)
+	       (eq (char-before) ?\;)))
+      (goto-char here))))
+
 (defun c-looking-at-inexpr-block (lim containing-sexp &optional check-at-end)
   ;; Return non-nil if we're looking at the beginning of a block
   ;; inside an expression.  The value returned is actually a cons of
@@ -10537,15 +10561,7 @@
 		    (and (c-major-mode-is 'c++-mode)
 			 (save-excursion
 			   (goto-char block-follows)
-			   (if (c-go-list-forward)
-			       (progn
-				 (backward-char)
-				 (c-syntactic-skip-backward
-				  "^;," block-follows t)
-				 (not (eq (char-before) ?\;)))
-			     (or (not (c-syntactic-re-search-forward
-				       "[;,]" nil t t))
-				 (not (eq (char-before) ?\;)))))))
+			   (not (c-looking-at-statement-block)))))
 		nil
 	      (cons 'inexpr-statement (point)))))
 
@@ -10871,6 +10887,14 @@
 		     (cdr (assoc (match-string 1)
 				 c-other-decl-block-key-in-symbols-alist))
 		     (max (c-point 'boi paren-pos) (point))))
+		   ((save-excursion
+		      (goto-char paren-pos)
+		      (c-looking-at-or-maybe-in-bracelist containing-sexp))
+		    (if (save-excursion
+			  (goto-char paren-pos)
+			  (c-looking-at-statement-block))
+			(c-add-syntax 'defun-block-intro nil)
+		      (c-add-syntax 'brace-list-intro nil)))
 		   (t (c-add-syntax 'defun-block-intro nil))))
 
 	      (c-add-syntax 'statement-block-intro nil)))
@@ -12692,7 +12716,10 @@
 	   nil))))
 
     (if (or (null res) (integerp res)
-	    (and (vectorp res) (= (length res) 1) (integerp (aref res 0))))
+	    (and (vectorp res) (= (length res) 1)
+		 (or (integerp (aref res 0))
+		     (and (consp (aref res 0))
+			  (integerp (car (aref res 0)))))))
 	res
       (c-benign-error "Error evaluating offset %S for %s: Got invalid value %S"
 		      offset symbol res)
@@ -12750,25 +12777,34 @@
     (while langelems
       (let* ((c-syntactic-element (car langelems))
 	     (res (c-calc-offset c-syntactic-element)))
-
-	(if (vectorp res)
-	    ;; Got an absolute column that overrides any indentation
-	    ;; we've collected so far, but not the relative
-	    ;; indentation we might get for the nested structures
-	    ;; further down the langelems list.
-	    (setq indent (elt res 0)
-		  anchor (point-min))	; A position at column 0.
-
+	(cond
+	 ((and (vectorp res) (consp (elt res 0)))
+	  ;; Got an absolute column that overrides any indentation we've
+	  ;; collected so far.  This can only be superseded by another
+	  ;; absolute column (another vector).
+	  (setq indent res
+		anchor (point-min)))	; A position at column 0.
+	 ((vectorp res)
+	  ;; Got an absolute column that overrides any indentation
+	  ;; we've collected so far, but not the relative
+	  ;; indentation we might get for the nested structures
+	  ;; further down the langelems list.
+	  (setq indent (elt res 0)
+		anchor (point-min)))	; A position at column 0.
+	 (t
 	  ;; Got a relative change of the current calculated
 	  ;; indentation.
-	  (setq indent (+ indent res))
+	  (if (not (vectorp indent))
+	      (setq indent (+ indent res)))
 
 	  ;; Use the anchor position from the first syntactic
 	  ;; element with one.
 	  (unless anchor
-	    (setq anchor (c-langelem-pos (car langelems)))))
+	    (setq anchor (c-langelem-pos (car langelems))))))
 
 	(setq langelems (cdr langelems))))
+    (if (vectorp indent)
+	(setq indent (car (elt indent 0))))
 
     (if anchor
 	(+ indent (save-excursion
diff -r ea743536b883 cc-styles.el
--- a/cc-styles.el	Fri Dec 30 15:19:21 2016 +0000
+++ b/cc-styles.el	Sun Jan 01 14:17:56 2017 +0000
@@ -72,6 +72,7 @@
 			 (arglist-close . c-lineup-arglist)
 			 (inline-open . 0)
 			 (brace-list-open . +)
+			 (brace-list-intro . c-lineup-arglist-intro-after-paren)
 			 (topmost-intro-cont
 			  . (first c-lineup-topmost-intro-cont
 				   c-lineup-gnu-DEFUN-intro-cont))))


> -- 
> Matthias

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#24431: 25.1.50; Wrong indentation in C++ mode when calling constructors with braces

[Alan Mackenzie]

Hello again, Matthias.

On Tue, Jan 17, 2017 at 21:28:58 +0100, Matthias Meulien wrote:
> Le dimanche 01 janvier 2017 à 14:32 +0000, Alan Mackenzie a écrit :

> I am sorry I am in the process of leaving my current living place and
> I've not found time to test your diff.  I don't know when I'll do it
> since I've so much work to do in my new apartment. Once more, I am
> really sorry...

Hope the move has gone/is going OK.

In the end, I discovered the patch I sent you was only half finished.
I've now finished the fix, and committed it, both to CC Mode and to Emacs
savannah.

I'd still be happy to hear of any testing you manage to get done on it,
particularly if anything is still buggy.

In the meantime, I'm closing the bug.

> -- 
> Matthias

-- 
Alan Mackenzie (Nuremberg, Germany).