[libcxx-commits] [PATCH] D108827: [libc++] Add an assertion in the subrange constructors with a size hint

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Sep 2 10:14:52 PDT 2021


ldionne added inline comments.


================
Comment at: libcxx/test/std/ranges/range.utility/range.subrange/types.h:55
 
 struct SizedSentinelForwardIter {
     typedef std::forward_iterator_tag            iterator_category;
----------------
Quuxplusone wrote:
> Scope creep: It's mildly weird that this type is called `SizedSentinelForwardIterator` when it doesn't actually involve a sentinel type. (Technically, this iterator type is its //own// sentinel type; but I call it mildly weird naming nonetheless.) It would be nicer to name it `SubtractableForwardIterator`.
> 
> As we fill out more of the C++20 library, you might also consider adding the sanity-checking static-asserts we couldn't originally do; e.g. on line 91, `static_assert(std::sized_sentinel_for<SubtractableForwardIterator, SubtractableForwardIterator>);`. This kind of assertion is best-practice in real life, to avoid hard-to-debug delayed error messages; I think it's reasonable to also put such assertions in our test-code helper headers. But scope creep, and also not worth fighting for right now.
I'll add the assertion but won't do the renaming here, it touches too many things for this PR. Feel free to submit a NFC patch with a naming you prefer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108827



More information about the libcxx-commits mailing list