[libcxx-commits] [PATCH] D99461: [libcxx] adds `std::indirectly_readable_traits` to <iterator>

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/readable.traits/indirectly_readable_traits.compile.pass.cpp:114
+
+static_assert(
+    check_explicit_member<possibly_different_cv_qualifiers<int, int>, int>());
----------------
cjdb wrote:
> ldionne wrote:
> > I know we don't want to spend too much time on formatting, however as I've said elsewhere, readability is still a major goal. I don't know if you'd agree, but this is very difficult to read IMO. The fact that we both use long identifier names and a not-super-long line wrap setting means that we get line breaks at random places. Aligning stuff would make the test easier to read, do you agree?
> > 
> > ```
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int, int>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int, int const>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int, int volatile>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int, int const volatile>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int const, int>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int const, int const>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int const, int volatile>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int const, int const volatile>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int volatile, int>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int volatile, int const>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int volatile, int volatile>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int volatile, int const volatile>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int const volatile, int>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int const volatile, int const>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int const volatile, int volatile>, int>());
> > static_assert(check_explicit_member<possibly_different_cv_qualifiers<int const volatile, int const volatile>, int>());
> > ```
> > 
> > Do you think there's a way to tweak the Clang format configuration to make this happen automatically?
> Yes, I can apply that to D99691 and then let clang-format take over. D99691 needs an LGTM though.
Feel free to tweak/remove the column limit in D99691 or in a separate patch. The only thing that's important for me is that I can read the code easily :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99461