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: Mon, 6 Nov 2023 10:47:03 +0000

Hello, Arsen.

Thanks for taking the trouble to report this bug, and thanks even more
for such a high quality bug report!

On Fri, Nov 03, 2023 at 10:51:32 +0100, Arsen Arsenović via CC-Mode-help wrote:
> Package: cc-mode

> <#secure method=pgpmime mode=sign>
> Package: cc-mode

> In the example below:

> namespace a {
> _BEGIN

>   template<>
>     struct foo
>     { /* HERE */
>     };
> }

> // Local Variables:
> // c-noise-macro-names: ("_BEGIN")
> // End:

> Pressing C-c C-s on the stat of the line marked HERE shows ((class-open
> 15)), which is the '_' of the '_BEGIN'.  This appears to be wrong, as
> the anchor should be the 'class' keyword on the line above, I believe.

> Anchoring '{' on the class/struct/union that precedes it would also
> enable implementing the GNU libstdc++ code style (as, AFAICT, it is
> currently impossible to indent a class with a basic-offset added to the
> column the 'template' keyword sits on).

Yes, in function c-looking-at-decl-block, the handling of template
constructs and noise macros was somewhat careless.  This should be fixed
by the patch below.

Even so, it's worth noting that noise macros with parentheses (set with
the buffer local variable c-noise-macro-with-parens-names) are currently
not handled correctly, here.  This was a decision taken some years ago,
to avoid slowing down CC Mode for a construct which is rarer than the
noise macros without parens.  I'm thinking of taking another look at
this.

Would you please apply the patch, byte compile cc-engine.el (or all of
CC Mode), load it into your Emacs and try it out on your real C++ code.
The patch should apply cleanly to the Emacs master branch.  Then please
confirm that it is actually fixes the bug, or tell me what's still not
working properly.  (If you want any help applying the patch or compiling
the file, feel free to send me private email.)

> Emacs  : GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.38, cairo version 1.17.8)
>  of 2023-09-24
> Package: CC Mode 5.35.2 (C++//l)
> Buffer Style: gnu
> c-emacs-features: (pps-extended-state col-0-paren posix-char-classes gen-string-delim gen-comment-delim syntax-properties category-properties 1-bit)

[ .... ]

diff -r 2760aada61fa cc-engine.el
--- a/cc-engine.el	Sun Oct 15 10:44:22 2023 +0000
+++ b/cc-engine.el	Mon Nov 06 10:26:34 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))))

> -- 
> Arsen Arsenović

-- 
Alan Mackenzie (Nuremberg, Germany).