Subject: Re: bug#72695: CC Mode 5.35.2 (C++//l); c++-mode misindents 'if constexpr' inside lambdas

From: Arsen Arsenović <[email-masked]>
Date: Sat, 21 Sep 2024 21:32:36 +0200

Hi Alan,

Alan Mackenzie <[email-masked]> writes:

> Hello, Arsen.
>
> On Tue, Sep 10, 2024 at 01:02:50 +0200, Arsen Arsenović wrote:
>> Hi Alan,
>
>> Alan Mackenzie <[email-masked]> writes:
>
> [ .... ]
>
> I'm hoping I've now fixed this bug (see patch below).

Thank you!

> [ .... ]
>
>> >> I've tried it on a few examples I came up with, it seems to work well in
>> >> 'real-life' code.
>
>> >> I've also constructed the following "stress test" which includes (almost
>> >> - see below) all the productions that can exist in a lambda function, in
>> >> the same context as the original bad example:
>
>> > Thanks, this was very useful.  As an important question, can and alignas
>> > specifier exist inside a lambda expression?  If so, what does it mean?
>> > My new patch below doesn't (yet) handle it.
>
>> It certainly can in the body.  I presume you mean in the
>> leader/declarator/whathaveyou however; in that case, yes, as part of the
>> attribute-specifier-seq in lambda-expression and lambda-declarator, and
>> its meaning depends on the position (see
>> https://eel.is/c++draft/expr.prim.lambda#closure-6).
>
> Thanks, I've now found the place in the C++ syntax where alignas can go
> in a lambda construct.  It is the only specifier which may go in these
> places, although any number of attributes are allowed.  (As a matter of
> interest, attributes in CC Mode are treated as syntactic whitespace.)
>
> [ .... ]
>
>> >>   void
>> >>   def ()
>> >>   {
>> >>     auto foo = [&,
>> >>   	      a,
>> >>   	      b = 123,
>> >>   	      c,
>> >>   	      ...d,
>> >>   	      ...e = 123
>> >>   	      ]
>> >>       <typename T,
>> >>        typename S>
>> >>       //alignas (128)
>> >>       [[gnu::noreturn]]
>> >>       requires some_concept<T>
>> >>       (int a,
>> >>        int b,
>> >>        int c)
>> >>       consteval
>> >>       constexpr
>> >>       mutable
>> >>       static
>> >>       //noexcept(true)
>> >>       [[gnu::packed]]
>> >>       //alignas (128)
>> >>       //->
>> >>       //int
>> >>       //requires (std::same_as <decltype (a), int>)
>> >>         {
>> >>       if constexpr (true)
>> >>         ;
>> >>     }
>> >>   }
>
> Question: Is that first (commented out) alignas correct where it is?  My
> reading of the version of the standard suggests that the alignas (128)
> should come _after_ the requires some_concept<T>, not before it.

I think so, through the 2nd lambda-expression derivation:

  lambda-introducer < template-parameter-list > requires-clause_opt
    attribute-specifier-seq_opt lambda-declarator compound-statement

... where attribute-specifier-seq is:

  attribute-specifier-seq_opt attribute-specifier

... and attribute-specifier is also possibly alignment-specifier.

Maybe I am missing something, though?

>> > Here's a modified version which is now fully handled (as far as it goes):
>
> And here's a further modified version, which my patch (below) now
> handles:
>
> void
> def ()
> {
>       auto foo = [&,
>               a,
>               b = 123,
>               c,
>               ...d,
>               ...e = 123
>       ]
>         <typename T,
>          typename S>
> //alignas (128)
>         [[gnu::noreturn]]
>         requires some_concept<T>
>         alignas (128)
>         (int a,
>          int b,
>          int c)
>         consteval
>         constexpr
>         mutable
>         static
>         noexcept(true)
>         [[gnu::packed]]
>         alignas (128)
>         -> int
>         requires (std::same_as <decltype (a), int>)
>       {
>         if constexpr (true)
>           ;
>       }
> }
>
> [ .... ]
>
>> >> Hopefully these examples help.
>
>> > Very much so.
>
>> >> Please also note that in a few places (such as alignas, the noexcept
>> >> specifier, RHS of assignments in the capture list) near-arbitrary
>> >> expressions can appear, and that requires clause have special boolean
>> >> expression syntax (see
>> >> https://eel.is/c++draft/temp.pre#nt:requires-clause).
>
>> > I'm afraid I haven't looked at this, yet.
>
> I have now.  :-)
>
> [ .... ]
>
>> > Here's an up to date version of the patch, which should work better than
>> > the last version.  Please do the usual with it, and let me know how it
>> > goes.  Thanks!
>
> And here's what I'm hoping will be the final version of the patch to fix
> this bug.  Again, could you try it out (on real code as well as test
> code), and let me know how well it works.  It should apply cleanly to the
> Emacs master branch.  Thanks!
>
> Even it the patch is good, I'm not quite ready to commit it yet, since I
> need to amend the test suite somewhat.

I will test it at a later time tonight or tomorrow.  Sorry for the
delay.

Thank you once again for your efforts!

Have a lovely evening!
-- 
Arsen Arsenović