[libcxx-commits] [PATCH] D116569: [libc++] [ranges] Add namespace __cpo to ranges::{advance, next, prev}.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__iterator/advance.h:74
+namespace __advance {
+  struct __fn final : private __function_like {
+  private:
----------------
var-const wrote:
> Formatting nit: I think namespaces [aren't supposed to](https://llvm.org/docs/CodingStandards.html#namespace-indentation) introduce a new level of indentation.
In general yeah, but this is consistent with most other niebloids/CPOs in libc++ today.
`git grep -h struct.__fn libcxx/include/`


================
Comment at: libcxx/include/__iterator/advance.h:75
+  struct __fn final : private __function_like {
+  private:
+    template <class _Tp>
----------------
var-const wrote:
> It looks like it's using tabs for indentation now, is this intentional?
On my browser screen I see little green >> chevrons, indicating that this all bumped over 2 spaces (intentionally); but that's not related to the choice of tabs/spaces.
In fact, CI will be unhappy with tabs, so if CI is green you can assume there are no tabs. Anyway, I'll double-check just to be sure, but I'm 99.99% sure that my editor is too dumb to insert tabs unintentionally. :)


================
Comment at: libcxx/include/__iterator/advance.h:192
+inline namespace __cpo {
+  inline constexpr auto advance = __advance::__fn(__function_like::__tag());
+} // namespace __cpo
----------------
var-const wrote:
> Same nit here: I think this variable declaration shouldn't be indented.
Again this is for consistency; and here (unlike above) we're almost 100% consistent with the indentation.
`git grep -h 'inline constexpr auto.*__fn' libcxx/include/`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116569