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

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Sep 27 16:55:27 PDT 2021


Quuxplusone added inline comments.


================
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>;
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110370



More information about the libcxx-commits mailing list