[libcxx-commits] [PATCH] D110503: [libc++] Implement P1394r4 for span: range constructor

[Joe Loser via Phabricator via libcxx-commits]

jloser marked an inline comment as done.
jloser added inline comments.


================
Comment at: libcxx/include/span:232
+      (void)__last;
+      _LIBCPP_ASSERT(__first <= __last, "invalid range in span's constructor (iterator, sentinel)");
+      _LIBCPP_ASSERT(__last - __first == _Extent,
----------------
Quuxplusone wrote:
> This has the same problem as in the `string_view` PR: I don't think `operator<=` is always valid here. Please add a test for an iterator/sentinel pair that doesn't support `<=`.
> 
> The issue is not that `__first <= __last` might be //false//; the issue is that it might not be well-formed at all.
The well-formedness is definitely a wrinkle. I'll add a test to expose this issue and remove the `_LIBCPP_ASSERT`.

Obviously, we could guard the `_LIBCPP_ASSERT` with `requires` to assure well-formedness of `__first <= __last`, but it's probably not worth it. I'd like to get some clarity in general on precondition checking in `libc++` as I mentioned in the `string_view` PR. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110503