On 24/09/20 09:04 -0400, Patrick Palka via Libstdc++ wrote:
The class template semiregular-box<T> defined in [range.semi.wrap] is used by a number of views to accomodate non-semiregular subobjects while ensuring that the overall view remains semiregular. It provides a stand-in default constructor, copy assignment operator and move assignment operator whenever the underlying type lacks them. The wrapper derives from std::optional<T> to support default construction when T is not default constructible.It would be nice for this wrapper to essentially be a no-op when the underlying type is already semiregular, but this is currently not the case due to its use of std::optional<T>, which incurs space overhead compared to storing just T. To that end, this patch specializes the semiregular wrapper for semiregular T. Compared to the primary template, this specialization uses less space and it allows [[no_unique_address]] to optimize away wrapped data members whose underlying type is empty and semiregular (e.g. a non-capturing lambda). This patch also applies [[no_unique_address]] to the five data members that currently use the wrapper. Tested on x86_64-pc-linux-gnu, does this look OK to commit? libstdc++-v3/ChangeLog: * include/std/ranges (__detail::__boxable): Split out the associated constraints of __box into here. (__detail::__box): Use the __boxable concept. Define a leaner partial specialization for semiregular types. (single_view::_M_value): Mark it [[no_unique_address]]. (filter_view::_M_pred): Likewise. (transform_view::_M_fun): Likewise. (take_while_view::_M_pred): Likewise. (drop_while_view::_M_pred):: Likewise. * testsuite/std/ranges/adaptors/detail/semiregular_box.cc: New test. --- libstdc++-v3/include/std/ranges | 68 +++++++++++++++-- .../ranges/adaptors/detail/semiregular_box.cc | 73 +++++++++++++++++++ 2 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/detail/semiregular_box.cc diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges index e7fa4493612..8a302a7918f 100644 --- a/libstdc++-v3/include/std/ranges +++ b/libstdc++-v3/include/std/ranges @@ -86,7 +86,10 @@ namespace ranges namespace __detail { - template<copy_constructible _Tp> requires is_object_v<_Tp> + template<typename _Tp> + concept __boxable = copy_constructible<_Tp> && is_object_v<_Tp>; + + template<__boxable _Tp> struct __box : std::optional<_Tp> { using std::optional<_Tp>::optional; @@ -130,6 +133,59 @@ namespace ranges } }; + // For types which are already semiregular, this specialization of the + // semiregular wrapper stores the object directly without going through + // std::optional. It provides the subset of the primary template's API + // that we currently use. + template<__boxable _Tp> requires semiregular<_Tp> + struct __box<_Tp> + { + private: + [[no_unique_address]] _Tp _M_value; + + public: + __box() = default; + + constexpr + __box(const _Tp& __t) + noexcept(is_nothrow_copy_constructible_v<_Tp>) + : _M_value{__t} + { } + + constexpr + __box(_Tp&& __t)
To be consistent with optional, these constructors should be conditionally explicit (and since we're in C++20 code here, we can actually use explicit(bool) rather than needing two overloads of each constructor). But I think we could just make them unconditionally explicit, since we only ever construct them explicitly. No need to allow implicit conversions if we never need them. Otherwise this looks great, p[lease push. It's an ABI change for the types using __box, so isn't appropriate for backporting to gcc-10 (unlike most changes to <ranges>).
