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: Thu, 16 Nov 2023 21:29:29 +0100
[Message part 1 (text/plain, inline)]
Hi Alan,
Thanks for working on this.
Alan Mackenzie <acm <at> muc.de> writes:
> Hello again, Arsen.
>
> On Mon, Nov 06, 2023 at 16:07:05 +0100, Arsen Arsenović wrote:
>
>> Alan Mackenzie <acm <at> muc.de> 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.
This behavior seems reasonable to me at a glance and at a test. I like
the idea :-)
>> > 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?
The following did work nicely (after I made c-langelem-2nd-col, by
copying and altering c-langelem-col as is obvious). I'm not sure if it
is idiomatic or the cleanest solution, though (plus, my Elisp-fu is
poor).
--8<---------------cut here---------------start------------->8---
(defun glibcxx-style/line-up-struct (sym-form)
"Lines up a class-open with its prior struct line"
(if (not (eq 'class-open (c-langelem-sym sym-form)))
nil
(let ((col (c-langelem-2nd-col c-syntactic-element t)))
(if col
(vector col)
nil))))
--8<---------------cut here---------------end--------------->8---
... however, I noticed that class-close still anchors on the template
that precedes it, and seems to not have a 2nd-pos to play with, so the
above line-up fn, when set on class-open, produces:
--8<---------------cut here---------------start------------->8---
template<typename foo>
struct f
{
};
--8<---------------cut here---------------end--------------->8---
... so, we're getting quite close!
(It could be that I'm missing something, of course..)
Thanks again, have a lovely night (-:
>> > 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ć
--
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.