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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/readable.traits/indirectly_readable_traits.pass.cpp:66
+  return result;
+}
+
----------------
cjdb wrote:
> Quuxplusone wrote:
> > This function should use regular `assert` and `return true;` at the end.
> See https://reviews.llvm.org/D99141#inline-935428.
I'm not sure what we get from using regular assert and returning true. We made this the standard quo for //updating// algorithms, etc. to be constexpr, because we wanted to preserve the existing test coverage in a constexpr context (i.e., both at runtime and during constexpr evaluation the same tests would be run), but that doesn't really apply here. These tests are only run in a constexpr context. 

In fact, I think it would actually be worse to use regular assert because that will give a confusing error and likely be much slower. (Regular assert will result in a "is not an integral constant" error whereas the static_assert will give the full stack trace, I think.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99461