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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__iterator/permutable.h:27-29
+    forward_iterator<_Iterator> &&
+    indirectly_movable_storable<_Iterator, _Iterator> &&
+    indirectly_swappable<_Iterator, _Iterator>;
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119222