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

Christopher Di Bella via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Apr 14 11:31:48 PDT 2021


cjdb 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>());
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99461



More information about the libcxx-commits mailing list