Package: cc-mode;
Reported by: Arsen Arsenović <arsen <at> aarsen.me>
Date: Fri, 3 Nov 2023 10:24:02 UTC
Severity: normal
Done: Alan Mackenzie <acm <at> muc.de>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Arsen Arsenović <arsen <at> aarsen.me> To: Alan Mackenzie <acm <at> muc.de> Cc: 66911 <at> debbugs.gnu.org Subject: bug#66911: CC Mode 5.35.2 (C++//l); Noise macro being taken as anchor to class-open Date: Mon, 06 Nov 2023 12:38:36 +0100
[Message part 1 (text/plain, inline)]
Hi Alan,
Alan Mackenzie <acm <at> muc.de> 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ć 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.)
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)
>
> - (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ć
--
Arsen Arsenović
[signature.asc (application/pgp-signature, inline)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.