[libcxx-commits] [PATCH] D110370: [libc++] Simplify std::ranges::subrange

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne marked an inline comment as done.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/include/__ranges/subrange.h:81
+    static constexpr bool _MustProvideSizeAtConstruction = !_StoreSize; // just to improve compiler diagnostics
+    struct _Empty { constexpr _Empty(auto) noexcept { } };
+    using _Size = conditional_t<_StoreSize, make_unsigned_t<iter_difference_t<_Iter>>, _Empty>;
----------------
Quuxplusone wrote:
> ldionne wrote:
> > Quuxplusone wrote:
> > > This type is not trivially default-constructible. I think you should give it a default ctor `_Empty() = default;` as well, and then
> > > ```
> > > [[no_unique_address]] _Size __size_ = _Size();
> > > ```
> > > and then your non-sized `subrange` type will be trivially default-constructible whenever the iterator+sentinel types are. And I think there should be a libcxx/ test for that, because it seems both useful //and// fragile.
> > I was going to say "awesome, let's do it and add a test for it". Then I tried, and noticed that `std::ranges::subrange<int*>` still wasn't trivially default constructible, since we have default member initializers per the spec (http://eel.is/c++draft/range.subrange.general).
> > 
> > So I'm not sure we can actually make `subrange` trivially default constructible, unless I am missing something.
> Well, `subrange<int*>` won't be trivially default constructible, but I suppose if you had some iterator type `It` such that value-initializing `It` were trivial, //then// it could be... hmm. Does having a NSDMI actually //prevent// the class from being trivially default constructible?!
> https://godbolt.org/z/1YEWvj71f
> Gross. Okay, well, I still like my `_Empty() = default` idea //if// it lets you get rid of the icky `_Empty(auto)` ctor template, but otherwise I guess I have no strong preference either way. I agree it's not going to help with triviality.
> Does having a NSDMI actually prevent the class from being trivially default constructible?!

Yeah, I was surprised by that too.

> Okay, well, I still like my `_Empty() = default` idea if it lets you get rid of the icky `_Empty(auto)` ctor template, but otherwise I guess I have no strong preference either way. I agree it's not going to help with triviality.

It doesn't because we have to construct `__size_(__n)` below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110370