[PATCH] D129443: [clang-format] Add option for aligning requires clause body

[Zhi-Hao Ye via Phabricator via cfe-commits]

Vigilans added a comment.

In my knowledge of clang-format, `Requires Clause` and `Requires Expression` 
are treated differently. 
In cppreference , A 
`Requires Clause` is specific to the `requires` keyword used for constraints on 
template arguments or on a function declaration. In this issue's context of 
continuation indent, the `requires` keyword is used for `Requires Expression`, 
just as the code relative to this diff suggests:

  if (Current.is(TT_RequiresExpression) /* <-- it's requires expression here */ 
&& Style.AlignRequiresClauseBody)
CurrentState.NestedBlockIndent = State.Column;

Some more example options in clang-format:

- `IndentRequiresClause`

  true:
  template 
requires Iterator
  void sort(It begin, It end) {
//
  }
  
  false:
  template 
  requires Iterator
  void sort(It begin, It end) {
//
  }

- `SpaceBeforeParensOptions -> AfterRequiresInClause`

  true:  false:
  templatevs.template
  requires (A && B)requires(A && B)
  ......

- `SpaceBeforeParensOptions -> AfterRequiresInExpression`

  true:  false:
  templatevs.template
  concept C = requires (T t) {   concept C = requires(T t) {
......
  }  }

So the option in this diff should use `RequiresExpressionBody`.

Besides, there is an option that does similar thing as this diff do:

- `LambdaBodyIndentation`

> The indentation style of lambda bodies. Signature (the default) causes the 
> lambda body to be indented one additional level relative to the indentation 
> level of the signature. OuterScope forces the lambda body to be indented one 
> additional level relative to the parent scope containing the lambda 
> signature. For callback-heavy code, it may improve readability to have the 
> signature indented two levels and to use OuterScope. The KJ style guide 
> requires OuterScope. KJ style guide
>
> Possible values:
>
> - LBI_Signature (in configuration: Signature) Align lambda body relative to 
> the lambda signature. This is the default.
>
>   someMethod(
>   [](SomeReallyLongLambdaSignatureArgument foo) {
> return;
>   });
>
> - LBI_OuterScope (in configuration: OuterScope) Align lambda body relative to 
> the indentation level of the outer scope the lambda signature resides in.
>
>   someMethod(
>   [](SomeReallyLongLambdaSignatureArgument foo) {
> return;
>   });

So we may select a similar option naming and values as this option?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129443/new/

https://reviews.llvm.org/D129443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

[Zhi-Hao Ye via Phabricator via cfe-commits]

Vigilans accepted this revision.
Vigilans added a comment.

In D129443#3844805 , 
@HazardyKnusperkeks wrote:

> In D129443#3844535 , @owenpan wrote:
>
>> In D129443#3842427 , 
>> @HazardyKnusperkeks wrote:
>>
>>> We can //fix// that regression, and I will adopt this change for my company 
>>> to keep the requires expressions where they are. :)
>>
>> Then we have three options:
>>
>> 1. Only fix the "regression".
>> 2. Add the option and make `OuterScope` the default.
>> 3. Add the option and make `Keyword` the default.
>>
>> @HazardyKnusperkeks I will defer to your choice. :)
>
> Add the option, and then either way we will have a regression to somebody. 
> `OuterScope` will regress to version 15. `Keyword` up to 14. So it should be 
> the most reasonable to make `OuterScope` the default, since the early 
> adapters will most be more open to change their configuration.
>
> @rymiel please add a change log note about that changed default.

I am fine with either default option with their persuasive reasons:

1. Default to `Keyword`: Interpreted as `Formally documented formatting 
behavior`, and the options arrangement looks more consistent with 
`LambdaBodyIndentation` (`Signature` (default) and `OuterScope`).
2. Default to `OuterScope`: Interpreted as `Fix the regression`, and the style 
looks more consistent with example codes in common concept tutorials (the 
expression is indented the same level as `concept` keyword, not `requires` 
keyword).

For consistency with `clang-format` current behaviors and options, we may 
prefer `Keyword`; For consistency with common current tutorial code, we may 
prefer `OuterScope`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129443/new/

https://reviews.llvm.org/D129443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits