[libcxx-commits] [PATCH] D142063: [libc++][ranges] implement `std::ranges::split_view` (WIP on testing, targeting llvm 16)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__ranges/split_view.h:120
+private:
+  bool __trailing_empty_                                        = false;
+  split_view<_View, _Pattern>* __parent_                        = nullptr;
----------------
huixie90 wrote:
> var-const wrote:
> > philnik wrote:
> > > Having this first seems like a bad idea. On 64-bit platforms this results in 7 bytes of padding. Maybe it would make sense to bit-pack this into the `split_view<_View, _Pattern>*`?
> > Also I don't think we have to follow the order in the standard, but there it's the last member variable.
> Yes I will move the bool to the end
> Could you please elaborate on bit-packing? 
We can make sure that `split_view` is always at least 2 byte aligned (which it is in most cases anyways), which guarantees that the low bit of the pointer is always zero. Since we know that the low bit is always zero, we can use it as a flag. We just have to bit-and the pointer if we want to dereference it. I have an implementation of a utility class for this purpose in D140278, but that's currently not `constexpr`-friendly and needs some more work in general. Using it would essentially mean that we can't ship `split_view` in LLVM16, but I think QoI is more important than having `split_view` now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142063