[libcxx-commits] [PATCH] D100278: [libcxx][iterator][ranges] adds `bidirectional_iterator` and `bidirectional_range`

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

Chris and I had a chat offline and we agreed that the `derived_from` part of the subsumption tests can be removed, and so the tests moved back to `test/std`. LGTM with those changes in.



================
Comment at: libcxx/test/libcxx/iterators/iterator.concepts/iterator.concept.bidir/subsumption.compile.pass.cpp:23
+requires std::derived_from<std::_ITER_CONCEPT<I>, std::bidirectional_iterator_tag>
+[[nodiscard]] constexpr bool check_subsumption() {
+  return false;
----------------
This is a nit, but I don't feel like adding `[[nodiscard]]` here adds anything. I would remove it here (and in all other places where we added it in similar places).

I think we need to clarify libc++'s policy for `[[nodiscard]]` to avoid marking every single function with it, since that gets into diminishing returns and actually makes stuff harder to read. You can check this in as-is though, and I'll make a NFC commit that changes it and other instances.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100278