On 21/11/19 13:33 -0500, JeanHeyd Meneide wrote:
This is an attempt to use concepts and constraints rather than
ranges::. It builds and run the tests I ran on x86_64 Linux decently
(albeit I'm still new at doing this). I wanted to get a feel for
whether or not this had been done right:
Thanks for doing this.
Thoughts?
Needs moar changelog.
diff --git a/libstdc++-v3/include/Makefile.am b/libstdc++-v3/include/Makefile.am
index 6300de9e96d..7f7035ca64e 100644
--- a/libstdc++-v3/include/Makefile.am
+++ b/libstdc++-v3/include/Makefile.am
@@ -156,6 +156,7 @@ bits_headers = \
${bits_srcdir}/random.tcc \
${bits_srcdir}/range_access.h \
${bits_srcdir}/range_cmp.h \
+ ${bits_srcdir}/range_concepts.h \
You've added this here, but not actually added a new header in the
patch.
index 3843ba5d57f..c4a4e92fed0 100644
--- a/libstdc++-v3/include/bits/iterator_concepts.h
+++ b/libstdc++-v3/include/bits/iterator_concepts.h
@@ -71,6 +71,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ *__t } -> __can_reference;
};
+ template<typename _LeftType, typename _RightType>
+ concept __is_compatible_pointer = is_convertible<_RightType(*)[],
_LeftType(*)[]>::value;
This could use is_convertible_v.
"left type" and "right type" don't
mean much to me, I'd prefer to keep it conventional and _Tp/_Up or
something meaninful like _From/_To.
I'd prefer not to have this at namespace-scope at all, at least not
unless we gie it a more descriptive name than __is_compatible_pointer.
How are they compatible? What is the context?
Something like array-compatible would be more descriptive, since
that's the condition it's testing, and the context it's used in.
std::unique_ptr<T[]> has similar checks so could reuse this ... except
we can't use concepts in std::unique_ptr. For that reason, I think an
alias template would be better, so it can be used in span and
unique_ptr<T[]> e.g.
template<typename _Tp, typename _Up>
using __array_compatible = is_convertible<_Tp(*)[], _Up(*)[]>;
That could be used in unique_ptr and can be used in span's constraints
too.
+
// FIXME: needed due to PR c++/67704
template<__detail::__dereferenceable _Tp>
struct __iter_ref
diff --git a/libstdc++-v3/include/bits/range_access.h
b/libstdc++-v3/include/bits/range_access.h
index de074460c16..66697ffc354 100644
--- a/libstdc++-v3/include/bits/range_access.h
+++ b/libstdc++-v3/include/bits/range_access.h
@@ -34,6 +34,7 @@
#if __cplusplus >= 201103L
#include <initializer_list>
+#include <concepts>
This is redundant, it's included by the next header anyway ...
#include <bits/iterator_concepts.h>
namespace std _GLIBCXX_VISIBILITY(default)
@@ -336,19 +337,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
ssize(const _Tp (&)[_Num]) noexcept
{ return _Num; }
- // "why are these in namespace std:: and not __gnu_cxx:: ?"
- // because if we don't put them here it's impossible to
- // have implicit ADL with "using std::begin/end/size/data;".
- template <typename _Container>
- constexpr auto
- __adl_begin(_Container& __cont) noexcept(noexcept(begin(__cont)))
- { return begin(__cont); }
-
- template <typename _Container>
- constexpr auto
- __adl_data(_Container& __cont) noexcept(noexcept(data(__cont)))
- { return data(__cont); }
-
#ifdef __cpp_lib_concepts
namespace ranges
{
@@ -869,10 +857,68 @@ namespace ranges
template<typename _Tp>
concept range = __detail::__range_impl<_Tp&>;
+ template<range _Range>
+ using sentinel_t = decltype(ranges::end(std::declval<_Range&>()));
+
+ template<range _Range>
+ using iterator_t = decltype(ranges::begin(std::declval<_Range&>()));
+
+ template<range _Range>
+ using range_value_t = iter_value_t<iterator_t<_Range>>;
+
+ template<range _Range>
+ using range_reference_t = iter_reference_t<iterator_t<_Range>>;
+
+ template<range _Range>
+ using range_rvalue_reference_t
+ = iter_rvalue_reference_t<iterator_t<_Range>>;
+
+ template<range _Range>
+ using range_difference_t = iter_difference_t<iterator_t<_Range>>;
+
+ namespace __detail
+ {
+ template<typename _Tp>
+ concept __forwarding_range = range<_Tp> && __range_impl<_Tp>;
+ } // namespace __detail
+
/// [range.sized] The sized_range concept.
template<typename _Tp>
concept sized_range = range<_Tp>
- && requires(_Tp& __t) { ranges::size(__t); };
+ && requires(_Tp& __t) { ranges::size(__t); };template<typename>
+ inline constexpr bool disable_sized_range = false;
+
+ // [range.refinements]
+ template<typename _Range, typename _Tp>
+ concept output_range
+ = range<_Range> && output_iterator<iterator_t<_Range>, _Tp>;
+
+ template<typename _Tp>
+ concept input_range = range<_Tp> && input_iterator<iterator_t<_Tp>>;
+
+ template<typename _Tp>
+ concept forward_range
+ = input_range<_Tp> && forward_iterator<iterator_t<_Tp>>;
+
+ template<typename _Tp>
+ concept bidirectional_range
+ = forward_range<_Tp> && bidirectional_iterator<iterator_t<_Tp>>;
+
+ template<typename _Tp>
+ concept random_access_range
+ = bidirectional_range<_Tp> && random_access_iterator<iterator_t<_Tp>>;
+
+ template<typename _Tp>
+ concept contiguous_range
+ = random_access_range<_Tp> && contiguous_iterator<iterator_t<_Tp>>
+ && requires(_Tp& __t)
+ {
+ { ranges::data(__t) } -> same_as<add_pointer_t<range_reference_t<_Tp>>>;
+ };
+
+ template<typename _Tp>
+ concept common_range
+ = range<_Tp> && same_as<iterator_t<_Tp>, sentinel_t<_Tp>>;
// [range.iter.ops] range iterator operations
@@ -1008,12 +1054,6 @@ namespace ranges
}
}
- template<range _Range>
- using iterator_t = decltype(ranges::begin(std::declval<_Range&>()));
-
- template<range _Range>
- using range_difference_t = iter_difference_t<iterator_t<_Range>>;
-
template<range _Range>
constexpr range_difference_t<_Range>
distance(_Range&& __r)
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 333d110b67e..a8b27764419 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -43,6 +43,7 @@
#include <iterator>
#include <limits>
#include <optional>
+#include <bits/range_concepts.h>
Here's that new header again, which isn't in your patch.
/**
* @defgroup ranges Ranges
@@ -55,66 +56,6 @@ namespace std _GLIBCXX_VISIBILITY(default)
_GLIBCXX_BEGIN_NAMESPACE_VERSION
namespace ranges
{
- // [range.range] The range concept.
- // Defined in <bits/range_iterator.h>
- // template<typename> concept range;
Please leave this comment (although I have just realised it gets the
name wrong ... oops).
diff --git a/libstdc++-v3/include/std/span b/libstdc++-v3/include/std/span
index fcec22a6c57..09864dc8e13 100644
--- a/libstdc++-v3/include/std/span
+++ b/libstdc++-v3/include/std/span
@@ -42,6 +42,7 @@
#include <tuple>
#include <utility>
#include <array>
+#include <concepts>
Redundant.
#include <bits/stl_iterator.h>
#include <bits/range_access.h>
@@ -105,6 +106,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
size_t _M_extent_value;
};
+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
+ // 3255. span's array constructor is too strict
+ template<typename _Type, size_t _Extent, typename _Tp, size_t _ArrayExtent>
+ concept __is_compatible_array
+ = (_Extent == dynamic_extent || _ArrayExtent == _Extent)
+ && __is_compatible_pointer<_Type, _Tp>;
+
+ template<typename _Type, typename _Iter>
+ concept __is_compatible_iterator
+ = contiguous_iterator<_Iter>
+ && is_lvalue_reference_v<iter_reference_t<_Iter>>
+ && same_as<iter_value_t<_Iter>,
remove_cvref_t<iter_reference_t<_Iter>>>
+ && __is_compatible_pointer<_Type,
remove_reference_t<iter_reference_t<_Iter>>>;
+
+ template<typename _Type, typename _Range>
+ concept __is_compatible_range
+ = __is_compatible_iterator<_Type, ranges::iterator_t<_Range>>;
} // namespace __detail
template<typename _Type, size_t _Extent = dynamic_extent>
@@ -122,21 +140,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return dynamic_extent;
}
- template<typename _Tp>
- using __is_compatible = is_convertible<_Tp(*)[], _Type(*)[]>;
-
- // _GLIBCXX_RESOLVE_LIB_DEFECTS
- // 3255. span's array constructor is too strict
- template<typename _Tp, size_t _ArrayExtent,
- typename = enable_if_t<_Extent == dynamic_extent
- || _ArrayExtent == _Extent>>
- using __is_compatible_array = __is_compatible<_Tp>;
The advantage of defining these here, not at namespace-scope, is that
you don't need to repeat the class template's parameters here.
-
public:
// member types
using value_type = remove_cv_t<_Type>;
using element_type = _Type;
- using index_type = size_t;
+ using size_type = size_t;
using reference = element_type&;
using const_reference = const element_type&;
using pointer = _Type*;
@@ -156,160 +164,74 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// constructors
- template<bool _DefaultConstructible = (_Extent + 1u) <= 1u,
- enable_if_t<_DefaultConstructible>* = nullptr>
- constexpr
- span() noexcept : _M_extent(0), _M_ptr(nullptr)
- { }
+ constexpr
+ span() noexcept
+ requires ((_Extent + 1u) <= 1u)
+ : _M_extent(0), _M_ptr(nullptr)
+ { }
constexpr
span(const span&) noexcept = default;
- template<typename _Tp, size_t _ArrayExtent,
- typename = _Require<__is_compatible_array<_Tp, _ArrayExtent>>>
+ template<typename _Tp, size_t _ArrayExtent>
+ requires __detail::__is_compatible_array<_Type, _Extent, _Tp,
_ArrayExtent>
constexpr
span(_Tp (&__arr)[_ArrayExtent]) noexcept
: span(static_cast<pointer>(__arr), _ArrayExtent)
{ }
- template<typename _Tp, size_t _ArrayExtent,
- typename = _Require<__is_compatible_array<_Tp, _ArrayExtent>>>
+ template<typename _Tp, size_t _ArrayExtent>
+ requires __detail::__is_compatible_array<_Type, _Extent, _Tp,
_ArrayExtent>
You only need four template arguments here because you defined it at
namespace-scope as a concept. What was wrong with the alias template?
You could have left the alias template and used it like:
requires __is_compatible_array<_Tp, _ArrayExtent>::value
constexpr
span(array<_Tp, _ArrayExtent>& __arr) noexcept
: span(static_cast<pointer>(__arr.data()), _ArrayExtent)
{ }
- template<typename _Tp, size_t _ArrayExtent,
- typename = _Require<__is_compatible_array<_Tp, _ArrayExtent>>>
+ template<typename _Tp, size_t _ArrayExtent>
+ requires __detail::__is_compatible_array<_Type, _Extent, const _Tp,
_ArrayExtent>
constexpr
span(const array<_Tp, _ArrayExtent>& __arr) noexcept
: span(static_cast<pointer>(__arr.data()), _ArrayExtent)
{ }
- // NOTE: when the time comes, and P1394 -
- // range constructors for std::span - ships in
- // the standard, delete the #else block and remove
- // the conditional
- // if the paper fails, delete #if block
- // and keep the crappy #else block
- // and then cry that NB comments failed C++20...
- // but maybe for C++23?
-#ifdef _GLIBCXX_P1394
- private:
- // FIXME: use std::iter_reference_t
- template<typename _Iterator>
- using iter_reference_t = decltype(*std::declval<_Iterator&>());
- // FIXME: use std::ranges::iterator_t
- // N.B. constraint is needed to prevent a cycle when __adl_begin finds
- // begin(span) which does overload resolution on span(Range&&).
- template<typename _Rng,
- typename _Rng2 = remove_cvref_t<_Rng>,
- typename = enable_if_t<!__detail::__is_std_span<_Rng2>::value>>
- using iterator_t = decltype(std::__adl_begin(std::declval<_Rng&>()));
- // FIXME: use std::iter_value_t
- template<typename _Iter>
- using iter_value_t = typename iterator_traits<_Iter>::value_type;
- // FIXME: use std::derived_from concept
- template<typename _Derived, typename _Base>
- using derived_from
- = __and_<is_base_of<_Base, _Derived>,
- is_convertible<const volatile _Derived*, const volatile _Base*>>;
- // FIXME: require contiguous_iterator<_Iterator>
- template<typename _Iter,
- typename _Ref = iter_reference_t<_Iter>,
- typename _Traits = iterator_traits<_Iter>,
- typename _Tag = typename _Traits::iterator_category>
- using __is_compatible_iterator
- = __and_<derived_from<_Tag, random_access_iterator_tag>,
- is_lvalue_reference<_Ref>,
- is_same<iter_value_t<_Iter>, remove_cvref_t<_Ref>>,
- __is_compatible<remove_reference_t<_Ref>>>;
-
- template<typename _Range>
- using __is_compatible_range
- = __is_compatible_iterator<iterator_t<_Range>>;
-
public:
- template<typename _Range, typename = _Require<
- bool_constant<_Extent == dynamic_extent>,
- __not_<__detail::__is_std_span<remove_cvref_t<_Range>>>,
- __not_<__detail::__is_std_array<remove_cvref_t<_Range>>>,
- __not_<is_array<remove_reference_t<_Range>>>,
- __is_compatible_range<_Range>>,
- typename = decltype(std::__adl_data(std::declval<_Range&>()))>
+ template<ranges::contiguous_range _Range>
+ requires (_Extent == dynamic_extent)
+ && (!__detail::__is_std_span<remove_cvref_t<_Range>>::value)
+ && (!__detail::__is_std_array<remove_cvref_t<_Range>>::value)
+ && (!is_array<remove_reference_t<_Range>>::value)
is_array_v<...> instead of is_array<...>::value
+ && __detail::__is_compatible_range<_Type, _Range>
constexpr
span(_Range&& __range)
- noexcept(noexcept(::std::__adl_data(__range))
- && noexcept(::std::__adl_size(__range)))
- : span(::std::__adl_data(__range), ::std::__adl_size(__range))
+ noexcept(noexcept(ranges::data(__range))
+ && noexcept(ranges::size(__range)))
+ : span(ranges::data(__range), ranges::size(__range))
{ }
- template<typename _ContiguousIterator, typename _Sentinel, typename
- = _Require<__not_<is_convertible<_Sentinel, index_type>>,
- __is_compatible_iterator<_ContiguousIterator>>>
+ template<contiguous_iterator _ContiguousIterator,
+ sized_sentinel_for<_ContiguousIterator> _Sentinel>
+ requires __detail::__is_compatible_iterator<_Type,
_ContiguousIterator>
+ && (!is_convertible<_Sentinel, size_type>::value)
is_convertible_v
