[libcxx-commits] [PATCH] D119222: [libc++][ranges] Implement `permutable`.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/__iterator/permutable.h:27-29
+    forward_iterator<_Iterator> &&
+    indirectly_movable_storable<_Iterator, _Iterator> &&
+    indirectly_swappable<_Iterator, _Iterator>;
----------------
Quuxplusone wrote:
> var-const wrote:
> > Quuxplusone wrote:
> > > Perhaps 2-space indent?
> > > Personally I'd use `_Ip` for the template parameter name, but whatever.
> > The indent width for split lines doesn't seem to be specified in the LLVM style guide, but clang-format thinks it's `4`: https://github.com/llvm/llvm-project/blob/07486395d2d05c9c567994456774cafdcc1611d0/clang/lib/Format/Format.cpp#L1163
> > I think there is some convenience in having regular indentation differ from indentation for split lines. If you feel otherwise, let me know.
> > I think there is some convenience in having regular indentation differ from indentation for split lines
> 
> Huh, I'd think the exact opposite, because the notion of what counts as "one line, split," versus "two lines," is completely fluid in C++. I don't see any logic to having 4-space indents on
> ```
> concept permutable =
>     x;
> ```
> but only 2-space indents on
> ```
> concept otherable =
>   requires (T x) {
>     x;
>   };
> ```
> I see LLVM's default style sets the various indent widths to `2` or `4` in what //I// would characterize as an arbitrary fashion, and maybe it's even worth nailing them all to `2` in our `.clang-format`.
> ```
>   LLVMStyle.IndentWidth = 2;
>   LLVMStyle.ObjCBlockIndentWidth = 2;
>   LLVMStyle.ConstructorInitializerIndentWidth = 4;
>   LLVMStyle.ContinuationIndentWidth = 4;
> ```
> So, yes I feel otherwise; but not a blocker.
I don't think these values are arbitrary -- the regular indent width is `2` (not sure why Objective-C blocks have a separate setting, but it makes sense for the code within blocks to be indented by `2` just like regular functions and lambdas) and the indent width for continuations is `4` (and they treat constructor initializer lists as continuations).

> but only 2-space indents on
It should be 
```
concept otherable =
    requires (T x) {
      x;
    };
```
Having `requires` on a separate line is a continuation; however, the contents of the `requires` clause should be formatted as usual, even though they are within a continuation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119222