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

2020-02-28 Thread Jonathan Wakely

On 27/02/20 00:02 +, 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 :
>
> 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 
Date:   Fri Feb 28 22:56:07 2020 +

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
-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
+using __maybe_present_t = conditional_t<_Present, _Tp, _Empty>;
 
+  // Alias for a type that is conditionally const.
   template
 using __maybe_const_t = conditional_t<_Const, const _Tp, _Tp>;
 
@@ -1065,8 +1070,8 @@ namespace views
   {
   protected:
 	[[no_unique_address]]
-	  __detail::__maybe_empty_t,
-_Callable> _M_callable;
+	  __detail::__maybe_present_t,
+  _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,
-  views::all_t<_InnerRange>> _M_inner;
+	__detail::__maybe_present_t,
+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,
-  iterator_t<_Base>> _M_current;
+	__detail::__maybe_present_t,
+	iterator_t<_Base>> _M_current;
 
 	public:
 	  using iterator_concept = conditional_t,
@@ -2969,7 +2975,7 @@ namespace views
 
   // XXX: _M_current is "present only if !forward_range"
   [[no_unique_address]]
-	__detail::__maybe_empty_t, iterator_t<_Vp>>
+	__detail::__maybe_present_t, iterator_t<_Vp>>
 	  _M_current;
 
 
@@ -3180,8 +3186,9 @@ namespace views
   static constexpr bool 

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

2020-02-26 Thread Jonathan Wakely
On Wed, 26 Feb 2020 at 18:31, Daniel Krügler wrote:
>
> Am Mi., 26. Feb. 2020 um 16:20 Uhr schrieb Jonathan Wakely 
> :
> >
> > 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.


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

2020-02-26 Thread Daniel Krügler
Am Mi., 26. Feb. 2020 um 16:20 Uhr schrieb Jonathan Wakely :
>
> 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


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

2020-02-26 Thread Jonathan Wakely
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 
Date:   Wed Feb 26 15:19:43 2020 +

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.

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
+using __maybe_empty_t = conditional_t<_NonEmpty, _Tp, _Empty>;
+
+  template
+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,
-   _Callable, __detail::_Empty> _M_callable;
+ __detail::__maybe_empty_t,
+   _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>;
 
  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>;
@@ -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,
- views::all_t<_InnerRange>, __detail::_Empty> _M_inner;
+   __detail::__maybe_empty_t,
+ views::all_t<_InnerRange>> _M_inner;
 
 public:
   join_view() = default;
@@ -2585,8 +2590,8 @@ namespace views
struct _OuterIter
{
private:
- using _Parent =