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

Konstantin Varlamov via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jan 4 13:18:14 PST 2022


var-const added inline comments.


================
Comment at: libcxx/include/__iterator/advance.h:74
+namespace __advance {
+  struct __fn final : private __function_like {
+  private:
----------------
Quuxplusone wrote:
> 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/`
I don't see a reason for niebloids to diverge from the style guide, and I don't think we should try to be consistent with things that are incorrectly (IMO) formatted. That would largely defeat the purpose of having a style guide if, once an inconsistent piece of code gets into the code base, it effectively takes precedence over the style guide.


================
Comment at: libcxx/include/__iterator/advance.h:75
+  struct __fn final : private __function_like {
+  private:
+    template <class _Tp>
----------------
Quuxplusone wrote:
> 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. :)
Yes, I took `>>` to mean tabs -- if this isn't the case, please disregard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116569



More information about the libcxx-commits mailing list