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

From: Arsen Arsenović <[email-masked]>
Date: Tue, 10 Sep 2024 01:02:50 +0200

Hi Alan,

Alan Mackenzie <[email-masked]> writes:

> Thanks!  I'm afraid I've spent all week on that dashed email setup,
> and it's now finally working.  So I've made only some progress on the
> bug, sorry.

No worries!

> But here is what I have:
>
> [ .... ]
>
>> Heh, given there's only a dozen or so syntax elements in Elisp, I agree
>> ;-).  C++ exposes lots of flexibility, so the syntax grew to match.
>
> Anyhow, I'm feeling less negative about C++'s syntax, now.

:-)

>> Lambda expressions declare both a data type and a function, and their
>> syntax exposes the syntax elements of both, hence the proverbial
>> complexity being the sum of the two.
>
> [ .... ]
>
>> > Anyhow, I've got a first version, incompletely tested, of a new
>> > handling for a lambda expression.  The patch is not nearly as big as
>> > it looks, two thirds of it is just re-indenting a large function I
>> > put an extra `if' form aroun.
>
>> 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).

In short: in the following:

  [] alignas (128) () alignas (64) {...}

... the operator() - i.e. the function containing the lambda body - of
the closure type will have alignment of 128 (whatever that means for
functions), and the function type itself will be have alignment of 64
(whatever that means for the function type).

>>   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)
>>         ;
>>     }
>>   }
>
> Here's a modified version which is now fully handled (as far as it goes):
>
>
> 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)
>           ;
>       }
> }
>
>
> [ .... ]
>
>> It seems that it gets confused by the trailing requires clauses, the
>> alignas, the noexcept and the trailing return type (all commented -
>> uncomment any or all to reproduce).
>
> These were coding errors in CC Mode.  I hope the patch below will have
> fixed these, apart from the alignas (see above).

Indeed that appears to be the case!

>> However, this does not appear to negatively impact the syntax
>> recognition instead, for the most part (I removed an element from the
>> stress test above in order to demonstrate that the issue with
>> indentation appears to be separate).
>
>> In the following example:
>
>>   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)
>>         ;
>>     }
>>   }
>
>> ... uncommenting the requires line confuses the indentation engine.
>
> I think this is fixed, now.
>
>> 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.

No worries.  This is somewhat akin to a few other constructs in C and
C++ where expressions can appear in an otherwise "type" context, so it
might already be handled (and the handling for concepts /could/ be
generic handling probably, without worrying about the special case in
the grammar - just something to consider).

>> Both of these pieces of syntax are reused in other contexts (other
>> declarations), so if there's a bug in them in the lambda case, it likely
>> carries over to the general declaration case.
>
>> Thank you again for maintaining CC-mode!  It is immeasurably useful.
>> And apologies for the trouble :-)
>
> No trouble at all!
>
> 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!

Seems to be working quite well in some contrived tests and a few
fragments of realistic looking code.

Thank you!

Have a lovely evening.
-- 
Arsen Arsenović