public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [committed] libstdc++: Remove precondition checks from ranges::subrange
@ 2021-06-15 18:27 Jonathan Wakely
  0 siblings, 0 replies; only message in thread
From: Jonathan Wakely @ 2021-06-15 18:27 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]

The assertion in the subrange constructor causes semantic changes,
because the call to ranges::distance performs additional operations that
are not part of the constructor's specification. That will fail to
compile if the iterator is move-only, because the argument to
ranges::distance is passed by value. It will modify the subrange if the
iterator is not a forward iterator, because incrementing the copy also
affects the _M_begin member. Those problems could be prevented by using
if-constexpr to only do the assertion for copyable forward iterators,
but the call to ranges::distance can also prevent the constructor being
usable in constant expressions. If the member initializers are usable in
constant expressions, but iterator increments of equality comparisons
are not, then the checks done by __glibcxx_assert might
make constant evaluation fail.

This change removes the assertion. Additionally, a new typedef is
introduced to simplify the declarations using __make_unsigned_like_t on
the iterator's difference type.

Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

libstdc++-v3/ChangeLog:

	* include/bits/ranges_util.h (subrange): Add __size_type typedef
	and use it to simplify declarations.
	(subrange(i, s, n)): Remove assertion.
	* testsuite/std/ranges/subrange/constexpr.cc: New test.

Tested powerpc64le-linux. Committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 5080 bytes --]

commit a88fc03ba7e52d9a072f25d42bb9619fedb7892e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jun 15 15:07:25 2021

    libstdc++: Remove precondition checks from ranges::subrange
    
    The assertion in the subrange constructor causes semantic changes,
    because the call to ranges::distance performs additional operations that
    are not part of the constructor's specification. That will fail to
    compile if the iterator is move-only, because the argument to
    ranges::distance is passed by value. It will modify the subrange if the
    iterator is not a forward iterator, because incrementing the copy also
    affects the _M_begin member. Those problems could be prevented by using
    if-constexpr to only do the assertion for copyable forward iterators,
    but the call to ranges::distance can also prevent the constructor being
    usable in constant expressions. If the member initializers are usable in
    constant expressions, but iterator increments of equality comparisons
    are not, then the checks done by __glibcxx_assert might
    make constant evaluation fail.
    
    This change removes the assertion. Additionally, a new typedef is
    introduced to simplify the declarations using __make_unsigned_like_t on
    the iterator's difference type.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/ranges_util.h (subrange): Add __size_type typedef
            and use it to simplify declarations.
            (subrange(i, s, n)): Remove assertion.
            * testsuite/std/ranges/subrange/constexpr.cc: New test.

diff --git a/libstdc++-v3/include/bits/ranges_util.h b/libstdc++-v3/include/bits/ranges_util.h
index b73fc121e0f..02561ee63f9 100644
--- a/libstdc++-v3/include/bits/ranges_util.h
+++ b/libstdc++-v3/include/bits/ranges_util.h
@@ -205,15 +205,18 @@ namespace ranges
       _It _M_begin = _It();
       [[no_unique_address]] _Sent _M_end = _Sent();
 
+      using __size_type
+	= __detail::__make_unsigned_like_t<iter_difference_t<_It>>;
+
       template<typename, bool = _S_store_size>
 	struct _Size
 	{ };
 
       template<typename _Tp>
 	struct _Size<_Tp, true>
-	{ __detail::__make_unsigned_like_t<_Tp> _M_size; };
+	{ _Tp _M_size; };
 
-      [[no_unique_address]] _Size<iter_difference_t<_It>> _M_size = {};
+      [[no_unique_address]] _Size<__size_type> _M_size = {};
 
     public:
       subrange() = default;
@@ -226,12 +229,10 @@ namespace ranges
 
       constexpr
       subrange(__detail::__convertible_to_non_slicing<_It> auto __i, _Sent __s,
-	       __detail::__make_unsigned_like_t<iter_difference_t<_It>> __n)
+	       __size_type __n)
 	requires (_Kind == subrange_kind::sized)
       : _M_begin(std::move(__i)), _M_end(__s)
       {
-	using __detail::__to_unsigned_like;
-	__glibcxx_assert(__n == __to_unsigned_like(ranges::distance(__i, __s)));
 	if constexpr (_S_store_size)
 	  _M_size._M_size = __n;
       }
@@ -258,8 +259,7 @@ namespace ranges
 	requires __detail::__convertible_to_non_slicing<iterator_t<_Rng>, _It>
 	  && convertible_to<sentinel_t<_Rng>, _Sent>
 	constexpr
-	subrange(_Rng&& __r,
-		 __detail::__make_unsigned_like_t<iter_difference_t<_It>> __n)
+	subrange(_Rng&& __r, __size_type __n)
 	requires (_Kind == subrange_kind::sized)
 	: subrange{ranges::begin(__r), ranges::end(__r), __n}
 	{ }
@@ -267,9 +267,9 @@ namespace ranges
       template<__detail::__not_same_as<subrange> _PairLike>
 	requires __detail::__pair_like_convertible_from<_PairLike, const _It&,
 							const _Sent&>
-      constexpr
-      operator _PairLike() const
-      { return _PairLike(_M_begin, _M_end); }
+	constexpr
+	operator _PairLike() const
+	{ return _PairLike(_M_begin, _M_end); }
 
       constexpr _It
       begin() const requires copyable<_It>
@@ -283,7 +283,7 @@ namespace ranges
 
       constexpr bool empty() const { return _M_begin == _M_end; }
 
-      constexpr __detail::__make_unsigned_like_t<iter_difference_t<_It>>
+      constexpr __size_type
       size() const requires (_Kind == subrange_kind::sized)
       {
 	if constexpr (_S_store_size)
diff --git a/libstdc++-v3/testsuite/std/ranges/subrange/constexpr.cc b/libstdc++-v3/testsuite/std/ranges/subrange/constexpr.cc
new file mode 100644
index 00000000000..f5bc52bef84
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/subrange/constexpr.cc
@@ -0,0 +1,26 @@
+// { dg-options "-std=gnu++20" }
+// { dg-do compile { target c++20 } }
+
+#include <ranges>
+
+struct iterator
+{
+  using difference_type = int;
+
+  int i;
+
+  int operator*() const { return i; }
+
+  // These are intentionally not constexpr:
+  iterator& operator++() { ++i; return *this; }
+  iterator operator++(int) { return {i++}; }
+  bool operator==(const iterator& it) const { return i == it.i; }
+};
+
+constexpr iterator begin(1), end(2);
+
+using std::ranges::subrange;
+using std::ranges::subrange_kind;
+
+// This used to fail due to using operator++ and operator== in an assertion:
+constexpr subrange<iterator, iterator, subrange_kind::sized> s(begin, end, 1);

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-06-15 18:28 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 18:27 [committed] libstdc++: Remove precondition checks from ranges::subrange Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).