[committed] libstdc++: Add __maybe_const_t and __maybe_empty_t aliases

Message ID 20200226152024.GA2443959@redhat.com
State New
Headers show
Series
  • [committed] libstdc++: Add __maybe_const_t and __maybe_empty_t aliases
Related show

Commit Message

Jonathan Wakely Feb. 26, 2020, 3:20 p.m.
This introduces a couple of convenience alias templates to be used for
some repeated patterns using std::conditional_t.

	* include/std/ranges (__detail::__maybe_empty_t): Define new helper
	alias.
	(__detail::__maybe_const_t): Likewise.
	(__adaptor::_RangeAdaptor): Use __maybe_empty_t.
	(transform_view, take_view, take_while_view, elements_view): Use
	__maybe_const_t.
	(join_view, split_view): Use both.

Tested powerpc64l-linux, committed to master.
commit 8017d95c7f55b98bcee1caf0216fdfd7fd613849
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Feb 26 15:19:43 2020 +0000

    libstdc++: Add __maybe_const_t and __maybe_empty_t aliases
    
    This introduces a couple of convenience alias templates to be used for
    some repeated patterns using std::conditional_t.
    
            * include/std/ranges (__detail::__maybe_empty_t): Define new helper
            alias.
            (__detail::__maybe_const_t): Likewise.
            (__adaptor::_RangeAdaptor): Use __maybe_empty_t.
            (transform_view, take_view, take_while_view, elements_view): Use
            __maybe_const_t.
            (join_view, split_view): Use both.

Comments

Daniel Krügler Feb. 26, 2020, 6:31 p.m. | #1
Am Mi., 26. Feb. 2020 um 16:20 Uhr schrieb Jonathan Wakely <jwakely@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.

Thanks,

- Daniel
Jonathan Wakely Feb. 27, 2020, 12:02 a.m. | #2
On Wed, 26 Feb 2020 at 18:31, Daniel Krügler wrote:
>

> Am Mi., 26. Feb. 2020 um 16:20 Uhr schrieb Jonathan Wakely <jwakely@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.
Jonathan Wakely Feb. 28, 2020, 11:08 p.m. | #3
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 <jwakely@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 <jwakely@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;

Patch

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index a7f4da957ef..d8326632166 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1029,6 +1029,13 @@  namespace views
 namespace __detail
 {
   struct _Empty { };
+
+  template<bool _NonEmpty, typename _Tp>
+    using __maybe_empty_t = conditional_t<_NonEmpty, _Tp, _Empty>;
+
+  template<bool _Const, typename _Tp>
+    using __maybe_const_t = conditional_t<_Const, const _Tp, _Tp>;
+
 } // namespace __detail
 
 namespace views
@@ -1058,8 +1065,8 @@  namespace views
       {
       protected:
 	[[no_unique_address]]
-	  conditional_t<!is_default_constructible_v<_Callable>,
-			_Callable, __detail::_Empty> _M_callable;
+	  __detail::__maybe_empty_t<!is_default_constructible_v<_Callable>,
+				    _Callable> _M_callable;
 
       public:
 	constexpr
@@ -1552,9 +1559,8 @@  namespace views
 	struct _Iterator
 	{
 	private:
-	  using _Parent
-	    = conditional_t<_Const, const transform_view, transform_view>;
-	  using _Base = conditional_t<_Const, const _Vp, _Vp>;
+	  using _Parent = __detail::__maybe_const_t<_Const, transform_view>;
+	  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
 
 	  static constexpr auto
 	  _S_iter_concept()
@@ -1760,9 +1766,8 @@  namespace views
 	struct _Sentinel
 	{
 	private:
-	  using _Parent
-	    = conditional_t<_Const, const transform_view, transform_view>;
-	  using _Base = conditional_t<_Const, const _Vp, _Vp>;
+	  using _Parent = __detail::__maybe_const_t<_Const, transform_view>;
+	  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
 
 	  constexpr range_difference_t<_Base>
 	  __distance_from(const _Iterator<_Const>& __i) const
@@ -1886,7 +1891,7 @@  namespace views
 	struct _Sentinel
 	{
 	private:
-	  using _Base = conditional_t<_Const, const _Vp, _Vp>;
+	  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
 	  using _CI = counted_iterator<iterator_t<_Base>>;
 
 	  sentinel_t<_Base> _M_end = sentinel_t<_Base>();
@@ -2025,7 +2030,7 @@  namespace views
 	struct _Sentinel
 	{
 	private:
-	  using _Base = conditional_t<_Const, const _Vp, _Vp>;
+	  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
 
 	  sentinel_t<_Base> _M_end = sentinel_t<_Base>();
 	  const _Pred* _M_pred = nullptr;
@@ -2258,8 +2263,8 @@  namespace views
 	struct _Iterator
 	{
 	private:
-	  using _Parent = conditional_t<_Const, const join_view, join_view>;
-	  using _Base = conditional_t<_Const, const _Vp, _Vp>;
+	  using _Parent = __detail::__maybe_const_t<_Const, join_view>;
+	  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
 
 	  static constexpr bool _S_ref_is_glvalue
 	    = is_reference_v<range_reference_t<_Base>>;
@@ -2450,8 +2455,8 @@  namespace views
 	struct _Sentinel
 	{
 	private:
-	  using _Parent = conditional_t<_Const, const join_view, join_view>;
-	  using _Base = conditional_t<_Const, const _Vp, _Vp>;
+	  using _Parent = __detail::__maybe_const_t<_Const, join_view>;
+	  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
 
 	  constexpr bool
 	  __equal(const _Iterator<_Const>& __i) const
@@ -2482,8 +2487,8 @@  namespace views
 
       // XXX: _M_inner is "present only when !is_reference_v<_InnerRange>"
       [[no_unique_address]]
-	conditional_t<!is_reference_v<_InnerRange>,
-		      views::all_t<_InnerRange>, __detail::_Empty> _M_inner;
+	__detail::__maybe_empty_t<!is_reference_v<_InnerRange>,
+				  views::all_t<_InnerRange>> _M_inner;
 
     public:
       join_view() = default;
@@ -2585,8 +2590,8 @@  namespace views
 	struct _OuterIter
 	{
 	private:
-	  using _Parent = conditional_t<_Const, const split_view, split_view>;
-	  using _Base = conditional_t<_Const, const _Vp, _Vp>;
+	  using _Parent = __detail::__maybe_const_t<_Const, split_view>;
+	  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
 
 	  constexpr bool
 	  __at_end() const
@@ -2618,8 +2623,8 @@  namespace views
 
 	  // XXX: _M_current is present only if "V models forward_range"
 	  [[no_unique_address]]
-	    conditional_t<forward_range<_Vp>,
-			  iterator_t<_Base>, __detail::_Empty> _M_current;
+	    __detail::__maybe_empty_t<forward_range<_Vp>,
+				      iterator_t<_Base>> _M_current;
 
 	public:
 	  using iterator_concept = conditional_t<forward_range<_Base>,
@@ -2732,7 +2737,7 @@  namespace views
 	struct _InnerIter
 	{
 	private:
-	  using _Base = conditional_t<_Const, const _Vp, _Vp>;
+	  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
 
 	  constexpr bool
 	  __at_end() const
@@ -2858,8 +2863,8 @@  namespace views
 
       // XXX: _M_current is "present only if !forward_range<V>"
       [[no_unique_address]]
-	conditional_t<!forward_range<_Vp>,
-		      iterator_t<_Vp>, __detail::_Empty> _M_current;
+	__detail::__maybe_empty_t<!forward_range<_Vp>, iterator_t<_Vp>>
+	  _M_current;
 
 
     public:
@@ -3223,7 +3228,7 @@  namespace views
       template<bool _Const>
 	struct _Iterator
 	{
-	  using _Base = conditional_t<_Const, const _Vp, _Vp>;
+	  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
 
 	  iterator_t<_Base> _M_current = iterator_t<_Base>();