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

From: Alan Mackenzie <[email-masked]>
Date: Sun, 1 Jan 2017 14:32:29 +0000

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).