Subject: Re: bug#66911: CC Mode 5.35.2 (C++//l); Noise macro being taken as anchor to class-open

From: Alan Mackenzie <[email-masked]>
Date: Thu, 16 Nov 2023 19:21:21 +0000

Hello again, Arsen.

On Mon, Nov 06, 2023 at 16:07:05 +0100, Arsen Arsenović wrote:

> Alan Mackenzie <[email-masked]> writes:

> >> Though, the anchor still anchors on the template<> line rather than the
> >> struct line in:

> >>   template<size_t _Nw>
> >>   struct _Base_bitset
> >>   {

> >> ... making it impossible (AFAICT) to create a style that implements:

> >>   template<size_t _Nw>
> >>     struct _Base_bitset
> >>     {
> >>     };

> >>   struct foo
> >>   {
> >>   };

> >> instead, the former gets formatted as:

> >>   template<size_t _Nw>
> >>     struct _Base_bitset
> >>   {
> >>   };

> >> Is this intended?

> > Apologies.  I missed this second point in your original bug report.

> Admittedly, I had considered making it two bug reports because the
> issues seemed separate to me, but I wasn't sure if one caused the other
> or vice-versa.  So, I don't blame you :-)

> > This should be fixable, but will be more work to fix than the other
> > point.

> > I think something like the following would do it: we could add a second
> > anchor point to topmost-intro-cont forms, that second point pointing at
> > struct.  In languages without templates/generics, or in constructs
> > lacking template components, both anchor points would be the same.

That was a mistake - the second anchor point is needed on class-open
lines, not topmost-intro-cont ones.

I've coded up this patch, which I include below.  Actually, only the last
two smallish hunks of the patch are new, the rest is what you've seen
before, so it will be necessary to start from a clean copy of
cc-engine.el again.

The syntactical context of the HERE line (the opening brace of that
class) is now something like:

    ((class-open 76 107))

, where 76 is the position of the template keyword and 107 that of the
class keyword.  When there's no template, both of these anchor points are
the same.  Actually, that's not quite accurate: the 107 is the point
after the indentation of the line that class is on; it just seemed better
that way.

> > So you could write an alignment function which would use that second
> > anchor point for class-open lines and the like.  Or, just possibly, we
> > might be able to make that second point the default for class-open.  This
> > should keep backward compatibility.  I'll have to think about this.

So, again, please let me know how you get on with this patch.  Is the
extra anchor point sufficient to write an alignment function to get the
indentation you want?

> > And of course, the manual and test programs will need amending, too.  ;-)

They still need amending.  On running the test suite, there were 81 files
where the new class-open syntactic symbol was different from the recorded
old symbol.  So I've got 81 files to regenerate.  Fortunately, I can do
this with a script, even if it will take me a couple of hours to write
the script.

> That sounds lovely - thanks in advance, if you do, indeed, decide to do
> it.  For now, I can get this patch implemented with some manual
> indenting.

> Thanks for the quick fix, have a lovely day :-)

Thank you indeed!  Have fun with the patch.

diff -r 2760aada61fa cc-engine.el
--- a/cc-engine.el	Sun Oct 15 10:44:22 2023 +0000
+++ b/cc-engine.el	Thu Nov 16 18:58:33 2023 +0000
@@ -12633,31 +12633,27 @@
   (let ((open-brace (point)) kwd-start first-specifier-pos)
     (c-syntactic-skip-backward c-block-prefix-charset limit t)
 
-    (when (and c-recognize-<>-arglists
-	       (eq (char-before) ?>))
-      ;; Could be at the end of a template arglist.
-      (let ((c-parse-and-markup-<>-arglists t))
-	(while (and
-		(c-backward-<>-arglist nil limit)
-		(progn
-		  (c-syntactic-skip-backward c-block-prefix-charset limit t)
-		  (eq (char-before) ?>))))))
-
-    ;; Skip back over noise clauses.
-    (while (and
-	    c-opt-cpp-prefix
-	    (eq (char-before) ?\))
-	    (let ((after-paren (point)))
-	      (if (and (c-go-list-backward)
-		       (progn (c-backward-syntactic-ws)
-			      (c-simple-skip-symbol-backward))
-		       (or (looking-at c-paren-nontype-key)
-			   (looking-at c-noise-macro-with-parens-name-re)))
-		  (progn
-		    (c-syntactic-skip-backward c-block-prefix-charset limit t)
-		    t)
-		(goto-char after-paren)
-		nil))))
+    (while
+	(or
+	 ;; Could be after a template arglist....
+	 (and c-recognize-<>-arglists
+	      (eq (char-before) ?>)
+	      (let ((c-parse-and-markup-<>-arglists t))
+		(c-backward-<>-arglist nil limit)))
+	 ;; .... or after a noise clause with parens.
+	 (and c-opt-cpp-prefix
+	      (let ((after-paren (point)))
+		(if (eq (char-before) ?\))
+		    (and
+		     (c-go-list-backward)
+		     (eq (char-after) ?\()
+		     (progn (c-backward-syntactic-ws)
+			    (c-simple-skip-symbol-backward))
+		     (or (looking-at c-paren-nontype-key) ; e.g. __attribute__
+			 (looking-at c-noise-macro-with-parens-name-re)))
+		  (goto-char after-paren)
+		  nil))))
+      (c-syntactic-skip-backward c-block-prefix-charset limit t))
 
     ;; Note: Can't get bogus hits inside template arglists below since they
     ;; have gotten paren syntax above.
@@ -12667,10 +12663,18 @@
 	   ;; The `c-decl-block-key' search continues from there since
 	   ;; we know it can't match earlier.
 	   (if goto-start
-	       (when (c-syntactic-re-search-forward c-symbol-start
-						    open-brace t t)
-		 (goto-char (setq first-specifier-pos (match-beginning 0)))
-		 t)
+	       (progn
+		 (while
+		     (and
+		      (c-syntactic-re-search-forward c-symbol-start
+						     open-brace t t)
+		      (goto-char (match-beginning 0))
+		      (if (or (looking-at c-noise-macro-name-re)
+			      (looking-at c-noise-macro-with-parens-name-re))
+			  (c-forward-noise-clause)
+			(setq first-specifier-pos (match-beginning 0))
+			nil)))
+		 first-specifier-pos)
 	     t)
 
 	   (cond
@@ -12739,34 +12743,39 @@
 	    (goto-char first-specifier-pos)
 
 	    (while (< (point) kwd-start)
-	      (if (looking-at c-symbol-key)
-		  ;; Accept any plain symbol token on the ground that
-		  ;; it's a specifier masked through a macro (just
-		  ;; like `c-forward-decl-or-cast-1' skip forward over
-		  ;; such tokens).
-		  ;;
-		  ;; Could be more restrictive wrt invalid keywords,
-		  ;; but that'd only occur in invalid code so there's
-		  ;; no use spending effort on it.
-		  (let ((end (match-end 0))
-			(kwd-sym (c-keyword-sym (match-string 0))))
-		    (unless
-			(and kwd-sym
-			     ;; Moving over a protection kwd and the following
-			     ;; ":" (in C++ Mode) to the next token could take
-			     ;; us all the way up to `kwd-start', leaving us
-			     ;; no chance to update `first-specifier-pos'.
-			     (not (c-keyword-member kwd-sym 'c-protection-kwds))
-			     (c-forward-keyword-clause 0))
-		      (goto-char end)
-		      (c-forward-syntactic-ws)))
-
+	      (cond
+	       ((or (looking-at c-noise-macro-name-re)
+		    (looking-at c-noise-macro-with-parens-name-re))
+		(c-forward-noise-clause))
+	       ((looking-at c-symbol-key)
+		;; Accept any plain symbol token on the ground that
+		;; it's a specifier masked through a macro (just
+		;; like `c-forward-decl-or-cast-1' skips forward over
+		;; such tokens).
+		;;
+		;; Could be more restrictive wrt invalid keywords,
+		;; but that'd only occur in invalid code so there's
+		;; no use spending effort on it.
+		(let ((end (match-end 0))
+		      (kwd-sym (c-keyword-sym (match-string 0))))
+		  (unless
+		      (and kwd-sym
+			   ;; Moving over a protection kwd and the following
+			   ;; ":" (in C++ Mode) to the next token could take
+			   ;; us all the way up to `kwd-start', leaving us
+			   ;; no chance to update `first-specifier-pos'.
+			   (not (c-keyword-member kwd-sym 'c-protection-kwds))
+			   (c-forward-keyword-clause 0))
+		    (goto-char end)
+		    (c-forward-syntactic-ws))))
+
+	       ((c-syntactic-re-search-forward c-symbol-start
+					       kwd-start 'move t)
 		;; Can't parse a declaration preamble and is still
 		;; before `kwd-start'.  That means `first-specifier-pos'
 		;; was in some earlier construct.  Search again.
-		(if (c-syntactic-re-search-forward c-symbol-start
-						   kwd-start 'move t)
-		    (goto-char (setq first-specifier-pos (match-beginning 0)))
+		(goto-char (setq first-specifier-pos (match-beginning 0))))
+	       (t
 		  ;; Got no preamble before the block declaration keyword.
 		  (setq first-specifier-pos kwd-start))))
 
@@ -14727,7 +14736,10 @@
 			    'lambda-intro-cont)))
 	(goto-char (cdr placeholder))
 	(back-to-indentation)
-	(c-add-stmt-syntax tmpsymbol nil t
+	(c-add-stmt-syntax tmpsymbol
+			   (and (eq tmpsymbol 'class-open)
+				(list (point)))
+			   t
 			   (c-most-enclosing-brace state-cache (point))
 			   paren-state)
 	(unless (eq (point) (cdr placeholder))
@@ -14770,9 +14782,10 @@
 	      (goto-char indent-point)
 	      (skip-chars-forward " \t")
 	      (and (eq (char-after) ?{)
-		   (c-looking-at-decl-block t)
+		   (setq tmp-pos (c-looking-at-decl-block t))
 		   (setq placeholder (point))))
-	    (c-add-syntax 'class-open placeholder))
+	    (c-add-syntax 'class-open placeholder
+			  (c-point 'boi tmp-pos)))
 
 	   ;; CASE 5A.3: brace list open
 	   ((save-excursion

> -- 
> Arsen Arsenović

-- 
Alan Mackenzie (Nuremberg, Germany).