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

--===============5378858470476179917==
Content-Type: multipart/signed; boundary="=-=-=";
 micalg=pgp-sha512; protocol="application/pgp-signature"

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Hi Alan,

Alan Mackenzie <[email protected]> writes:

> 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=C4=87 via CC-Mode-=
help wrote:
>> Package: cc-mode
>
>> <#secure method=3Dpgpmime mode=3Dsign>
>> 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.)

That indeed appears to have fixed it.  I built emacs 30.0.50 from git
(94807b6896191245ff3bef44a0ec21efb918232f) and applied your patch, then
compared the analysis and indentation before and after on libstdc++es
<bitset>, and it worked as expected (the noise macro stopped confusing
it).

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?

>> 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)
>=20=20
> -    (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=20
> +	 ;; 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))
>=20=20
>      ;; Note: Can't get bogus hits inside template arglists below since t=
hey
>      ;; 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)
>=20=20
>  	   (cond
> @@ -12739,34 +12743,39 @@
>  	    (goto-char first-specifier-pos)
>=20=20
>  	    (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))))
>=20=20
>
>> --=20
>> Arsen Arsenovi=C4=87


=2D-=20
Arsen Arsenovi=C4=87

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iOYEARYKAI4WIQT+4rPRE/wAoxYtYGFSwpQwHqLEkwUCZUjVHV8UgAAAAAAuAChp
c3N1ZXItZnByQG5vdGF0aW9ucy5vcGVucGdwLmZpZnRoaG9yc2VtYW4ubmV0RkVF
MkIzRDExM0ZDMDBBMzE2MkQ2MDYxNTJDMjk0MzAxRUEyQzQ5MxAcYXJzZW5AYWFy
c2VuLm1lAAoJEFLClDAeosSTFYIBAPX3dUxuFPSN8dcVgSYn+CR68Fx7utfMjHhP
TGH5tqbjAP9ujXKfIIvO0Dls27eWty+lE95iucL67XrlKM61ns4gBA==
=oB88
-----END PGP SIGNATURE-----
--=-=-=--




--===============5378858470476179917==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline


--===============5378858470476179917==--