* Re: [committed] libstdc++: Fix invalid noexcept-specifier (PR 94117)
2020-03-10 11:40 [committed] libstdc++: Fix invalid noexcept-specifier (PR 94117) Jonathan Wakely
@ 2020-03-10 17:52 ` Jonathan Wakely
2020-03-10 22:16 ` Jonathan Wakely
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Wakely @ 2020-03-10 17:52 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 814 bytes --]
On 10/03/20 11:40 +0000, Jonathan Wakely wrote:
>G++ fails to diagnose this non-dependent expression, but Clang doesn't
>like it.
>
> PR c++/94117
> * include/std/ranges (ranges::transform_view::_Iterator::iter_move):
> Change expression in noexcept-specifier to match function body.
>
This patch goes further and removes the __iter_move helper completely,
and the __iter_swap one, in transform_view.
It also does the same in split_view, and fixes a bug where the
noexcept-specifier was always false.
I've also added new _M_i_current() accessors (overloaded for const and
non-const) to return _M_i.__current(). Using this instead of
_M_i._M_current fixes a bug in inner-iterator::operator*() (which is
also present in the working draft).
Tested powerpc64le-linux, committed to master.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 7402 bytes --]
commit cf0c3a457319df1e8dc9321227162a7c57169a39
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Mar 10 17:45:45 2020 +0000
libstdc++: Fix noexcept guarantees for ranges::split_view
Also introduce the _M_i_current() accessors to solve the problem of
access to the private member of _OuterIter from the iter_move and
iter_swap overloads (which are only friends of _InnerIter not
_OuterIter).
* include/std/ranges (transform_view::_Iterator::__iter_move): Remove.
(transform_view::_Iterator::operator*): Add noexcept-specifier.
(transform_view::_Iterator::iter_move): Inline __iter_move body here.
(split_view::_OuterIter::__current): Add noexcept.
(split_view::_InnerIter::__iter_swap): Remove.
(split_view::_InnerIter::__iter_move): Remove.
(split_view::_InnerIter::_M_i_current): New accessors.
(split_view::_InnerIter::__at_end): Use _M_i_current().
(split_view::_InnerIter::operator*): Likewise.
(split_view::_InnerIter::operator++): Likewise.
(iter_move(const _InnerIter&)): Likewise.
(iter_swap(const _InnerIter&, const _InnerIter&)): Likewise.
* testsuite/std/ranges/adaptors/split.cc: Check noexcept-specifier
for iter_move and iter_swap on split_view's inner iterator.
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 292132db990..4dc7342e2f7 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1679,17 +1679,6 @@ namespace views
return input_iterator_tag{};
}
- static constexpr decltype(auto)
- __iter_move(const _Iterator& __i = {})
- noexcept(noexcept(std::__invoke(*__i._M_parent->_M_fun,
- *__i._M_current)))
- {
- if constexpr (is_lvalue_reference_v<decltype(*__i)>)
- return std::move(*__i);
- else
- return *__i;
- }
-
using _Base_iter = iterator_t<_Base>;
_Base_iter _M_current = _Base_iter();
@@ -1728,6 +1717,7 @@ namespace views
constexpr decltype(auto)
operator*() const
+ noexcept(noexcept(std::__invoke(*_M_parent->_M_fun, *_M_current)))
{ return std::__invoke(*_M_parent->_M_fun, *_M_current); }
constexpr _Iterator&
@@ -1837,8 +1827,13 @@ namespace views
{ return __x._M_current - __y._M_current; }
friend constexpr decltype(auto)
- iter_move(const _Iterator& __i) noexcept(noexcept(__iter_move(__i)))
- { return __iter_move(__i); }
+ iter_move(const _Iterator& __i) noexcept(noexcept(*__i))
+ {
+ if constexpr (is_lvalue_reference_v<decltype(*__i)>)
+ return std::move(*__i);
+ else
+ return *__i;
+ }
friend constexpr void
iter_swap(const _Iterator& __x, const _Iterator& __y)
@@ -2715,7 +2710,7 @@ namespace views
// current of outer-iterator. current is equivalent to current_ if
// V models forward_range, and parent_->current_ otherwise.
constexpr auto&
- __current()
+ __current() noexcept
{
if constexpr (forward_range<_Vp>)
return _M_current;
@@ -2724,7 +2719,7 @@ namespace views
}
constexpr auto&
- __current() const
+ __current() const noexcept
{
if constexpr (forward_range<_Vp>)
return _M_current;
@@ -2860,7 +2855,7 @@ namespace views
auto __end = ranges::end(_M_i._M_parent->_M_base);
if constexpr (__detail::__tiny_range<_Pattern>)
{
- const auto& __cur = _M_i.__current();
+ const auto& __cur = _M_i_current();
if (__cur == __end)
return true;
if (__pcur == __pend)
@@ -2869,7 +2864,7 @@ namespace views
}
else
{
- auto __cur = _M_i.__current();
+ auto __cur = _M_i_current();
if (__cur == __end)
return true;
if (__pcur == __pend)
@@ -2896,16 +2891,13 @@ namespace views
return _Cat{};
}
- static constexpr decltype(auto)
- __iter_move(const _InnerIter& __i = {})
- noexcept(noexcept(ranges::iter_move(__i._M_i.__current())))
- { return ranges::iter_move(__i._M_i.__current()); }
+ constexpr auto&
+ _M_i_current() noexcept
+ { return _M_i.__current(); }
- static constexpr void
- __iter_swap(const _InnerIter& __x = {}, const _InnerIter& __y = {})
- noexcept(noexcept(ranges::iter_swap(__x._M_i.__current(),
- __y._M_i.__current())))
- { ranges::iter_swap(__x._M_i.__current(), __y._M_i.__current()); }
+ constexpr auto&
+ _M_i_current() const noexcept
+ { return _M_i.__current(); }
_OuterIter<_Const> _M_i = _OuterIter<_Const>();
bool _M_incremented = false;
@@ -2926,7 +2918,7 @@ namespace views
constexpr decltype(auto)
operator*() const
- { return *_M_i._M_current; }
+ { return *_M_i_current(); }
constexpr _InnerIter&
operator++()
@@ -2935,7 +2927,7 @@ namespace views
if constexpr (!forward_range<_Base>)
if constexpr (_Pattern::size() == 0)
return *this;
- ++_M_i.__current();
+ ++_M_i_current();
return *this;
}
@@ -2962,14 +2954,16 @@ namespace views
{ return __x.__at_end(); }
friend constexpr decltype(auto)
- iter_move(const _InnerIter& __i) noexcept(noexcept(__iter_move()))
- { return __iter_move(__i); }
+ iter_move(const _InnerIter& __i)
+ noexcept(noexcept(ranges::iter_move(__i._M_i_current())))
+ { return ranges::iter_move(__i._M_i_current()); }
friend constexpr void
iter_swap(const _InnerIter& __x, const _InnerIter& __y)
- noexcept(noexcept(__iter_swap()))
+ noexcept(noexcept(ranges::iter_swap(__x._M_i_current(),
+ __y._M_i_current())))
requires indirectly_swappable<iterator_t<_Base>>
- { __iter_swap(__x, __y); }
+ { ranges::iter_swap(__x._M_i_current(), __y._M_i_current()); }
};
_Vp _M_base = _Vp();
@@ -3010,8 +3004,8 @@ namespace views
begin()
{
if constexpr (forward_range<_Vp>)
- return _OuterIter<__detail::__simple_view<_Vp>>{*this,
- ranges::begin(_M_base)};
+ return _OuterIter<__detail::__simple_view<_Vp>>{
+ *this, ranges::begin(_M_base)};
else
{
_M_current = ranges::begin(_M_base);
@@ -3028,7 +3022,8 @@ namespace views
constexpr auto
end() requires forward_range<_Vp> && common_range<_Vp>
{
- return _OuterIter<__detail::__simple_view<_Vp>>{*this, ranges::end(_M_base)};
+ return _OuterIter<__detail::__simple_view<_Vp>>{
+ *this, ranges::end(_M_base)};
}
constexpr auto
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
index e7556725e4f..abdbfb41d8b 100644
--- a/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
@@ -121,6 +121,18 @@ test06()
b = ranges::begin(v);
}
+void
+test07()
+{
+ char str[] = "banana split";
+ auto split = str | views::split(' ');
+ auto val = *split.begin();
+ auto b = val.begin();
+ auto b2 = b++;
+ static_assert( noexcept(iter_move(b)) );
+ static_assert( noexcept(iter_swap(b, b2)) );
+}
+
int
main()
{
@@ -130,4 +142,5 @@ main()
test04();
test05();
test06();
+ test07();
}
^ permalink raw reply [flat|nested] 3+ messages in thread