Subject: Re: bug#22486: CC Mode 5.33 (C++/l); Bad indentation in a hideous C++ struct

From: Alan Mackenzie <[email-masked]>
Date: Sun, 14 Feb 2016 16:55:52 +0000

Hello, Michael.

On Fri, Feb 05, 2016 at 02:01:52PM -0500, Michael Welsh Duggan wrote:
> Alan Mackenzie <[email-masked]> writes:

> > On Thu, Jan 28, 2016 at 03:37:07PM -0500, Michael Welsh Duggan wrote:
> >> In the following C++ snippet:

> >> template <typename Arg, typename... Args>
> >> struct ArgListMatcher<Arg, Args...> :
> >>     ArgListMatcher<MakeIndices<CountRef<Arg>::value>,
> >>                    MakeIndices<sizeof...(Args) - CountRef<Arg>::value, CountRef<Arg>::value>,
> >>                        Arg, Args...>    <============================
> >> {
> >>   using Parent = ArgListMatcher<
> >>     MakeIndices<CountRef<Arg>::value>,
> >>     MakeIndices<sizeof...(Args) + 1 - CountRef<Arg>::value,
> >>                 CountRef<Arg>::value>, Arg, Args...>;
> >>   using Parent::ArgListMatcher;
> >> };

> >> Starting with "template" as the first line, the lines' contexts are
> >> listed as:

> >> syntax: ((topmost-intro 1)), indent: 0
> >> syntax: ((topmost-intro-cont 1)), indent: 0
> >> syntax: ((inher-intro 1)), indent: 4
> >> syntax: ((template-args-cont 1 99)), indent: 19
> >> syntax: ((member-init-intro 154)), indent: 23  <=====================
> >> syntax: ((defun-open 1)), indent: 0

> >> All of these seem reasonable save for member-init-intro, which I
> >> expected to be template-args-cont.

Please see the patch below.  Basically, I've had to slacken the criteria
for detecting template delimiters, so the danger of mistaking a
less-than followed by a greater-than for a template are higher than they
were.  Come to think of it, the danger probably isn't that high anyway.

Would you please do the usual with the patch and let me know how well
the problem is solved in real code.  Thanks!

[ .... ]

> Yeah.  You're going to see arithmetic operators in template parameter
> lists a lot more due to the way variadic templates are handled.  (They
> basically are type-matched recursively via partial template
> specialization.)

Maybe C++ compilers are going to mandate the handling of tail recursion.
:-)

[ .... ]

> > Would it, perhaps, be feasible to detect the ... operator as an
> > indication that +, -, (?etc.) are permissible inside template delimiters?
> > Is it true that ... only exists inside either a variadic parameter list
> > or template delimiters?

> Nope, sorry.  Functions can use the ... as well.  Here's an example,
> compilable with g++ -std=c++11:

>   #include <iostream>
  
>   int sum()
>   {
>       return 0;
>   }
  
>   template<typename... T>
>   int sum(int first, T... rest)
>   {
>       return first + sum(rest...);
>   }
  
>   int sum2()
>   {
>       return 0;
>   }
  
>   template<typename... T>
>   int sum2(int first, T... rest)
>   {
>       return first + sum2(1 + rest...);
>   }
  
>   int main()
>   {
>       std::cout << sum(1, 2, 3, 4) << std::endl;  /* outputs 10 */
>       std::cout << sum2(1, 2, 3, 4) << std::endl; /* outputs 16 */
>   }

> sum(1, 2, 3, 4) turns into (1 + 2 + 3 + 4 + 0), whereas sum2(1, 2, 3, 4)
> turns into (1 + 3 + 5 + 7 + 0).  This is because of the sum(rest...) and
> sum2(1 + rest...) expansions.

> sum(rest...) turns into sum(rest0, rest1, ..., restn)
> sum(1 + rest...) turns into sum (1 + rest0, 1 + rest1, ..., 1 + restn)

Well it's interesting that C++ can now have functions with a genuinely
variable number of arguments, rather than the clumsy mechanism (which C
still needs to use) in <varargs.h> (or whatever that include file is
called).  The cost is high, though.

Anyhow, here's the patch:

diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index a4a1604..d4dcb1c 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -6056,7 +6056,10 @@ c-forward-<>-arglist-recur
 		   ;; Stop on ',', '|', '&', '+' and '-' to catch
 		   ;; common binary operators that could be between
 		   ;; two comparison expressions "a<b" and "c>d".
-		   "[<;{},|+&-]\\|[>)]"
+		   ;; 2016-02-11: C++11 templates can now contain arithmetic
+		   ;; expressions, so template detection in C++ is now less
+		   ;; robust than it was.
+		   c-<>-notable-chars-re
 		   nil t t))
 
 		(cond
@@ -6064,7 +6067,9 @@ c-forward-<>-arglist-recur
 		  ;; Either an operator starting with '>' or the end of
 		  ;; the angle bracket arglist.
 
-		  (if (looking-at c->-op-without->-cont-regexp)
+		  (if (save-excursion
+			(c-backward-token-2)
+			(looking-at c-multichar->-op-not->>-regexp))
 		      (progn
 			(goto-char (match-end 0))
 			t)		; Continue the loop.
@@ -6134,6 +6139,11 @@ c-forward-<>-arglist-recur
 		      )))
 		  t)			; carry on looping.
 
+		 ((and
+		   (eq (char-before) ?\()
+		   (c-go-up-list-forward)
+		   (eq (char-before) ?\))))
+
 		 ((and (not c-restricted-<>-arglists)
 		       (or (and (eq (char-before) ?&)
 				(not (eq (char-after) ?&)))
diff --git a/lisp/progmodes/cc-langs.el b/lisp/progmodes/cc-langs.el
index 8a1d43c..9f1dd1d 100644
--- a/lisp/progmodes/cc-langs.el
+++ b/lisp/progmodes/cc-langs.el
@@ -228,6 +228,12 @@ c-lang-defvar
     ;; with the group symbol for each group and should return non-nil
     ;; if that group is to be included.
     ;;
+    ;; OP-FILTER selects the operators.  It is either t to select all
+    ;; operators, a string to select all operators for which `string-match'
+    ;; matches the operator with the string, or a function which will be
+    ;; called with the operator and should return non-nil when the operator
+    ;; is to be selected.
+    ;;
     ;; If XLATE is given, it's a function which is called for each
     ;; matching operator and its return value is collected instead.
     ;; If it returns a list, the elements are spliced directly into
@@ -1245,7 +1251,6 @@ 'c-opt-op-identitier-prefix
 		    t
 		    "\\`<."
 		    (lambda (op) (substring op 1)))))
-
 (c-lang-defvar c-<-op-cont-regexp (c-lang-const c-<-op-cont-regexp))
 
 (c-lang-defconst c->-op-cont-tokens
@@ -1264,7 +1269,6 @@ 'c-opt-op-identitier-prefix
   ;; Regexp matching the second and subsequent characters of all
   ;; multicharacter tokens that begin with ">".
   t (c-make-keywords-re nil (c-lang-const c->-op-cont-tokens)))
-
 (c-lang-defvar c->-op-cont-regexp (c-lang-const c->-op-cont-regexp))
 
 (c-lang-defconst c->-op-without->-cont-regexp
@@ -1279,10 +1283,19 @@ 'c-opt-op-identitier-prefix
 		     "\\`>>"
 		     (lambda (op) (substring op 1)))
        :test 'string-equal)))
-
 (c-lang-defvar c->-op-without->-cont-regexp
   (c-lang-const c->-op-without->-cont-regexp))
 
+(c-lang-defconst c-multichar->-op-not->>-regexp
+  ;; Regexp matching multichar tokens containing ">", except ">>"
+  t (c-make-keywords-re nil
+      (delete ">>"
+	      (c-filter-ops (c-lang-const c-all-op-syntax-tokens)
+			    t
+			    "\\(.>\\|>.\\)"))))
+(c-lang-defvar c-multichar->-op-not->>-regexp
+  (c-lang-const c-multichar->-op-not->>-regexp))
+
 (c-lang-defconst c-stmt-delim-chars
   ;; The characters that should be considered to bound statements.  To
   ;; optimize `c-crosses-statement-barrier-p' somewhat, it's assumed to
@@ -3087,6 +3100,19 @@ 'c-opt-op-identitier-prefix
 		    ; generics is not yet coded in CC Mode.
 (c-lang-defvar c-recognize-<>-arglists (c-lang-const c-recognize-<>-arglists))
 
+(c-lang-defconst c-<>-notable-chars-re
+  "A regexp matching any single character notable inside a <...> construct.
+This must include \"<\" and \">\", \",\", and any character which cannot be
+valid inside such a construct.  This is used in `c-forward-<>-arglist-recur' to
+try to detect sequences of tokens which cannot be a template \(etc.)
+construct.  When \"(\" is present, that defun will attempt to parse a
+parenthesized expression inside the template.  When \")\" is present it will
+treat an unbalanced closing paren as a sign of the invalidity of the
+putative template construct."
+  t "[<;{},|+&->)]"
+  c++ "[<;{},>()]")
+(c-lang-defvar c-<>-notable-chars-re (c-lang-const c-<>-notable-chars-re))
+
 (c-lang-defconst c-enums-contain-decls
   "Non-nil means that an enum structure can contain declarations."
   t nil

> -- 
> Michael Welsh Duggan
> ([email-masked])

-- 
Alan Mackenzie (Nuremberg, Germany).