[libcxx-commits] [PATCH] D102468: [libcxx][ranges] removes default_initializable from weakly_incrementable and view

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

Ship-it. I'll be looking forward to a patch that makes `insert_iterator` up-to-spec for C++20.



================
Comment at: libcxx/include/iterator:919
     _Container* container;
-    typename _Container::iterator iter;
+    typename _Container::iterator iter; // FIXME: `ranges::iterator_t<Container>` in C++20 mode
 public:
----------------
ldionne wrote:
> zoecarver wrote:
> > Quuxplusone wrote:
> > > cjdb wrote:
> > > > ldionne wrote:
> > > > > Why a FIXME instead of `#if _LIBCPP_STD_VER > XXXX`?
> > > > > 
> > > > > If both types exist, then they should be the same, so I don't see any sort of ABI issue in case that was the concern.
> > > > I want to put this in a more relevant patch.
> > > I agree with Louis's assessment here.
> > Theoretically there's a section in the ranges status document to track things like this... but I guess we don't really use it.
> > 
> > Anyway, please make sure you have this on some todo list that you're going to look at at some point (or put it in the ranges status doc). Make sure it's not just a browser tab ;)
> Actually, I think this should be
> 
> ```
> #if _LIBCPP_SUPPORTS_RANGES
> ranges::iterator_t<Container> iter;
> #else
>  typename _Container::iterator iter;
> #endif
> ```
> 
> I don't see a reason to wait for another patch?
As discussed offline, this will be tackled in a separate patch that makes `insert_iterator` fully up-to-spec for C++20.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102468