[libcxx-commits] [PATCH] D119057: [libc++][ranges] Implement rbegin, rend, crbegin and crend.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__ranges/rbegin.h:28
+
+#if !defined(_LIBCPP_HAS_NO_CONCEPTS)
+
----------------
var-const wrote:
> Quuxplusone wrote:
> > ldionne wrote:
> > > This should be guarded by `HAS_NO_INCOMPLETE_RANGES` too (and other headers too).
> > Yes (perhaps), but that's D118736; I think as long as this header is consistent with what we //currently// do, it's better not to introduce potential churn over it here.
> > 
> > My impression of D118736 is that we're still kinda confused about what we want `HAS_NO_INCOMPLETE_RANGES` to encompass — I believe in my original PR it would have been included because it's in `ranges::`; and then in my subsequent tentative proposal it wouldn't have been because it has no ABI implications (no struct layout); and then in your current proposal it would again be included because it's in `ranges::` //and// no `std::` concept depends on it. I assume we'll talk about that offline soon. :)
> My reasoning was pretty much what @Quuxplusone said -- until D118736 lands, I think new code should follow the existing approach. Do you think we should start applying `HAS_NO_INCOMPLETE_RANGES` now, without waiting for that patch?
I agree. I just spoke with Arthur offline and we agreed on what we needed to do -- we'll try to do it, but the burden shouldn't be on this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119057