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

From: Arsen Arsenović <[email-masked]>
Date: Thu, 16 Nov 2023 21:29:29 +0100

Hi Alan,

Thanks for working on this.

Alan Mackenzie <[email-masked]> writes:

> 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.

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ć