On 27/02/20 00:02 +0000, Jonathan Wakely wrote:
On Wed, 26 Feb 2020 at 18:31, Daniel Krügler wrote:
Am Mi., 26. Feb. 2020 um 16:20 Uhr schrieb Jonathan Wakely <jwak...@redhat.com>:
>
> This introduces a couple of convenience alias templates to be used for
> some repeated patterns using std::conditional_t.
I find it a bit confusing/inconsistent to define __maybe_const_t such
that the bool argument says "is const", while for __maybe_empty_t the
corresponding bool argument says "is not empty". Suggestion: Implement
__maybe_empty_t such that its bool argument says "is empty" or change
the alias to __maybe_nonempty_t.
Good point, I'll make that change. Thanks.
Here's what I'm proposing to change it to. The name "maybe present"
matches the relevant wording in the working draft, fixes the reversed
logic w.r.t the bool argument, and doesn't refer to the fact that the
type may be empty (which is not really the salient point about it).
While preparing this change it occurred to me that all types using
this alias might end up with a member of the same __detail::_Empty
type. If two such types would otherwise be at the same address, the
_Empty data member would force them to have different addresses. I
think in practice that's not an issue, because the only class template
that uses __maybe_present_t without any other data members is
_RangeAdaptor, and I don't think we need to care about users doing
something like:
struct Wtf : decltype(std::views::filter), decltype(std::views::take)
{ };
With the current code the two base classes can't have the same
address, because they each have a single data member of type
__detail::_Empty, which must have unique addresses.
It would be possible to implement those range adaptors so that the two
base subobjects could be at the same address, and so sizeof(Wtf) would
be smaller. I don't think I care about that.
commit 43cab9a6e5ea3b2b31ce6b82074d09bfd764640c
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Fri Feb 28 22:56:07 2020 +0000
libstdc++: Rename __detail::__maybe_empty_t alias template
The key property of this alias is not that it may be an empty type, but
that the type argument may not be used. The fact it's replaced by an
empty type is just an implementation detail. The name was also
backwards with respect to the bool argument.
This patch changes the name to better reflect its purpose.
* include/std/ranges (__detail::__maybe_empty_t): Rename to
__maybe_present_t.
(__adaptor::_RangeAdaptor, join_view, split_view): Use new name.
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 19d3da950e7..c71cf918cfc 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1030,9 +1030,14 @@ namespace __detail
{
struct _Empty { };
- template<bool _NonEmpty, typename _Tp>
- using __maybe_empty_t = conditional_t<_NonEmpty, _Tp, _Empty>;
+ // Alias for a type that is conditionally present
+ // (and is an empty type otherwise).
+ // Data members using this alias should use [[no_unique_address]] so that
+ // they take no space when not needed.
+ template<bool _Present, typename _Tp>
+ using __maybe_present_t = conditional_t<_Present, _Tp, _Empty>;
+ // Alias for a type that is conditionally const.
template<bool _Const, typename _Tp>
using __maybe_const_t = conditional_t<_Const, const _Tp, _Tp>;
@@ -1065,8 +1070,8 @@ namespace views
{
protected:
[[no_unique_address]]
- __detail::__maybe_empty_t<!is_default_constructible_v<_Callable>,
- _Callable> _M_callable;
+ __detail::__maybe_present_t<!is_default_constructible_v<_Callable>,
+ _Callable> _M_callable;
public:
constexpr
@@ -2211,8 +2216,9 @@ namespace views
static constexpr bool _S_needs_cached_begin = !random_access_range<_Vp>;
[[no_unique_address]]
- __detail::__maybe_empty_t<_S_needs_cached_begin,
- __detail::_CachedPosition<_Vp>> _M_cached_begin;
+ __detail::__maybe_present_t<_S_needs_cached_begin,
+ __detail::_CachedPosition<_Vp>>
+ _M_cached_begin;
public:
drop_view() = default;
@@ -2592,8 +2598,8 @@ namespace views
// XXX: _M_inner is "present only when !is_reference_v<_InnerRange>"
[[no_unique_address]]
- __detail::__maybe_empty_t<!is_reference_v<_InnerRange>,
- views::all_t<_InnerRange>> _M_inner;
+ __detail::__maybe_present_t<!is_reference_v<_InnerRange>,
+ views::all_t<_InnerRange>> _M_inner;
public:
join_view() = default;
@@ -2728,8 +2734,8 @@ namespace views
// XXX: _M_current is present only if "V models forward_range"
[[no_unique_address]]
- __detail::__maybe_empty_t<forward_range<_Vp>,
- iterator_t<_Base>> _M_current;
+ __detail::__maybe_present_t<forward_range<_Vp>,
+ iterator_t<_Base>> _M_current;
public:
using iterator_concept = conditional_t<forward_range<_Base>,
@@ -2969,7 +2975,7 @@ namespace views
// XXX: _M_current is "present only if !forward_range<V>"
[[no_unique_address]]
- __detail::__maybe_empty_t<!forward_range<_Vp>, iterator_t<_Vp>>
+ __detail::__maybe_present_t<!forward_range<_Vp>, iterator_t<_Vp>>
_M_current;
@@ -3180,8 +3186,9 @@ namespace views
static constexpr bool _S_needs_cached_begin
= !common_range<_Vp> && !random_access_range<_Vp>;
[[no_unique_address]]
- __detail::__maybe_empty_t<_S_needs_cached_begin,
- __detail::_CachedPosition<_Vp>> _M_cached_begin;
+ __detail::__maybe_present_t<_S_needs_cached_begin,
+ __detail::_CachedPosition<_Vp>>
+ _M_cached_begin;
public:
reverse_view() = default;
