From: Jonathan Wakely <jwakely@redhat.com>
To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: Re: [committed] libstdc++: Move attributes that follow requires-clauses [PR101782]
Date: Thu, 5 Aug 2021 19:02:16 +0100 [thread overview]
Message-ID: <YQwnqPykYypmI45n@redhat.com> (raw)
In-Reply-To: <YQv4cAPOBuj2y5Ra@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]
On 05/08/21 15:40 +0100, Jonathan Wakely wrote:
>On 05/08/21 15:19 +0100, Jonathan Wakely wrote:
>>On 04/08/21 12:55 +0100, Jonathan Wakely wrote:
>>>This adds [[nodiscard]] throughout <iterator>, as proposed by P2377R0
>>>(with some minor corrections).
>>>
>>>The attribute is added for all modes from C++11 up, using
>>>[[__nodiscard__]] or _GLIBCXX_NODISCARD where C++17 [[nodiscard]] can't
>>>be used directly.
>>
>>This change causes errors when -fconcepts-ts is used. Fixed like so.
>>
>>Tested powerpc64le-linux, committed to trunk.
>>
>
>>commit 7b1de3eb9ed3f8dde54732d88520292c5ad1157d
>>Author: Jonathan Wakely <jwakely@redhat.com>
>>Date: Thu Aug 5 13:34:00 2021
>>
>> libstdc++: Move attributes that follow requires-clauses [PR101782]
>> As explained in the PR, the grammar in the Concepts TS means that a [
>> token following a requires-clause is parsed as part of the
>> logical-or-expression rather than the start of an attribute. That makes
>> the following ill-formed when using -fconcepts-ts:
>> template<typename T> requires foo<T> [[nodiscard]] int f(T);
>> This change moves all attributes that follow a requires-clause to the
>> end of the function declarator.
>
>
>Except that as Jakub pointed out, putting it there doesn't work.
>
>It needs to be:
>
> template<typename T> requires foo<T> int f [[nodiscard]] (T);
>
>At least the testsuite isn't failing now, but the attributes I moved
>have no effect. I'll fix it ... some time.
This should be correct now.
Tested powerpc64le-linux, pushed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 14051 bytes --]
commit c8b024fa4b76bfd914e96dd3cecfbb6ee8e91316
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Aug 5 16:46:00 2021
libstdc++: Move [[nodiscard]] attributes again [PR101782]
Where I moved these nodiscard attributes to made them apply to the
function type, not to the function. This meant they no longer generated
the desired -Wunused-result warnings, and were ill-formed with Clang
(but only a pedwarn with GCC).
Clang also detected ill-formed attributes in <queue> which this fixes.
Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
libstdc++-v3/ChangeLog:
PR libstdc++/101782
* include/bits/ranges_base.h (ranges::begin, ranges::end)
(ranges::rbegin, ranges::rend, ranges::size, ranges::ssize)
(ranges::empty, ranges::data): Move attribute after the
declarator-id instead of at the end of the declarator.
* include/bits/stl_iterator.h (__gnu_cxx::__normal_iterator):
Move attributes back to the start of the function declarator,
but move the requires-clause to the end.
(common_iterator): Move attribute after the declarator-id.
* include/bits/stl_queue.h (queue): Remove ill-formed attributes
from friend declaration that are not definitions.
* include/std/ranges (views::all, views::filter)
(views::transform, views::take, views::take_while,
views::drop) (views::drop_while, views::join,
views::lazy_split) (views::split, views::counted,
views::common, views::reverse) (views::elements): Move
attributes after the declarator-id.
diff --git a/libstdc++-v3/include/bits/ranges_base.h b/libstdc++-v3/include/bits/ranges_base.h
index 1dac9680b4f..49c7d9c9f06 100644
--- a/libstdc++-v3/include/bits/ranges_base.h
+++ b/libstdc++-v3/include/bits/ranges_base.h
@@ -111,8 +111,7 @@ namespace ranges
requires is_array_v<remove_reference_t<_Tp>> || __member_begin<_Tp>
|| __adl_begin<_Tp>
constexpr auto
- operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
- [[nodiscard]]
+ operator()[[nodiscard]](_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
{
if constexpr (is_array_v<remove_reference_t<_Tp>>)
{
@@ -163,8 +162,7 @@ namespace ranges
requires is_bounded_array_v<remove_reference_t<_Tp>>
|| __member_end<_Tp> || __adl_end<_Tp>
constexpr auto
- operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
- [[nodiscard]]
+ operator()[[nodiscard]](_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
{
if constexpr (is_bounded_array_v<remove_reference_t<_Tp>>)
{
@@ -268,9 +266,8 @@ namespace ranges
template<__maybe_borrowed_range _Tp>
requires __member_rbegin<_Tp> || __adl_rbegin<_Tp> || __reversable<_Tp>
constexpr auto
- operator()(_Tp&& __t) const
+ operator()[[nodiscard]](_Tp&& __t) const
noexcept(_S_noexcept<_Tp&>())
- [[nodiscard]]
{
if constexpr (__member_rbegin<_Tp>)
return __t.rbegin();
@@ -327,9 +324,8 @@ namespace ranges
template<__maybe_borrowed_range _Tp>
requires __member_rend<_Tp> || __adl_rend<_Tp> || __reversable<_Tp>
constexpr auto
- operator()(_Tp&& __t) const
+ operator()[[nodiscard]](_Tp&& __t) const
noexcept(_S_noexcept<_Tp&>())
- [[nodiscard]]
{
if constexpr (__member_rend<_Tp>)
return __t.rend();
@@ -417,8 +413,7 @@ namespace ranges
requires is_bounded_array_v<remove_reference_t<_Tp>>
|| __member_size<_Tp> || __adl_size<_Tp> || __sentinel_size<_Tp>
constexpr auto
- operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
- [[nodiscard]]
+ operator()[[nodiscard]](_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
{
if constexpr (is_bounded_array_v<remove_reference_t<_Tp>>)
return extent_v<remove_reference_t<_Tp>>;
@@ -438,8 +433,7 @@ namespace ranges
template<typename _Tp>
requires requires (_Tp& __t) { _Size{}(__t); }
constexpr auto
- operator()(_Tp&& __t) const noexcept(noexcept(_Size{}(__t)))
- [[nodiscard]]
+ operator()[[nodiscard]](_Tp&& __t) const noexcept(noexcept(_Size{}(__t)))
{
auto __size = _Size{}(__t);
using __size_type = decltype(__size);
@@ -498,8 +492,7 @@ namespace ranges
requires __member_empty<_Tp> || __size0_empty<_Tp>
|| __eq_iter_empty<_Tp>
constexpr bool
- operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
- [[nodiscard]]
+ operator()[[nodiscard]](_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
{
if constexpr (__member_empty<_Tp>)
return bool(__t.empty());
@@ -540,8 +533,7 @@ namespace ranges
template<__maybe_borrowed_range _Tp>
requires __member_data<_Tp> || __begin_data<_Tp>
constexpr auto
- operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp>())
- [[nodiscard]]
+ operator()[[nodiscard]](_Tp&& __t) const noexcept(_S_noexcept<_Tp>())
{
if constexpr (__member_data<_Tp>)
return __t.data();
diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 053ae41e9c3..c5b02408c1c 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -1118,21 +1118,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#if __cpp_lib_three_way_comparison
template<typename _IteratorL, typename _IteratorR, typename _Container>
- requires requires (_IteratorL __lhs, _IteratorR __rhs)
- { { __lhs == __rhs } -> std::convertible_to<bool>; }
+ [[nodiscard]]
constexpr bool
operator==(const __normal_iterator<_IteratorL, _Container>& __lhs,
const __normal_iterator<_IteratorR, _Container>& __rhs)
noexcept(noexcept(__lhs.base() == __rhs.base()))
- [[nodiscard]]
+ requires requires {
+ { __lhs.base() == __rhs.base() } -> std::convertible_to<bool>;
+ }
{ return __lhs.base() == __rhs.base(); }
template<typename _IteratorL, typename _IteratorR, typename _Container>
+ [[nodiscard]]
constexpr std::__detail::__synth3way_t<_IteratorR, _IteratorL>
operator<=>(const __normal_iterator<_IteratorL, _Container>& __lhs,
const __normal_iterator<_IteratorR, _Container>& __rhs)
noexcept(noexcept(std::__detail::__synth3way(__lhs.base(), __rhs.base())))
- [[nodiscard]]
{ return std::__detail::__synth3way(__lhs.base(), __rhs.base()); }
#else
// Forward iterator requirements
@@ -1985,9 +1986,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _It2, sentinel_for<_It> _Sent2>
requires sentinel_for<_Sent, _It2>
friend bool
- operator==(const common_iterator& __x,
- const common_iterator<_It2, _Sent2>& __y)
- [[nodiscard]]
+ operator== [[nodiscard]] (const common_iterator& __x,
+ const common_iterator<_It2, _Sent2>& __y)
{
switch(__x._M_index << 2 | __y._M_index)
{
@@ -2008,9 +2008,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _It2, sentinel_for<_It> _Sent2>
requires sentinel_for<_Sent, _It2> && equality_comparable_with<_It, _It2>
friend bool
- operator==(const common_iterator& __x,
- const common_iterator<_It2, _Sent2>& __y)
- [[nodiscard]]
+ operator== [[nodiscard]] (const common_iterator& __x,
+ const common_iterator<_It2, _Sent2>& __y)
{
switch(__x._M_index << 2 | __y._M_index)
{
@@ -2032,9 +2031,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<sized_sentinel_for<_It> _It2, sized_sentinel_for<_It> _Sent2>
requires sized_sentinel_for<_Sent, _It2>
friend iter_difference_t<_It2>
- operator-(const common_iterator& __x,
- const common_iterator<_It2, _Sent2>& __y)
- [[nodiscard]]
+ operator- [[nodiscard]] (const common_iterator& __x,
+ const common_iterator<_It2, _Sent2>& __y)
{
switch(__x._M_index << 2 | __y._M_index)
{
diff --git a/libstdc++-v3/include/bits/stl_queue.h b/libstdc++-v3/include/bits/stl_queue.h
index 363868fe0a6..41ffc50d380 100644
--- a/libstdc++-v3/include/bits/stl_queue.h
+++ b/libstdc++-v3/include/bits/stl_queue.h
@@ -107,18 +107,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#endif
template<typename _Tp1, typename _Seq1>
- _GLIBCXX_NODISCARD
friend bool
operator==(const queue<_Tp1, _Seq1>&, const queue<_Tp1, _Seq1>&);
template<typename _Tp1, typename _Seq1>
- _GLIBCXX_NODISCARD
friend bool
operator<(const queue<_Tp1, _Seq1>&, const queue<_Tp1, _Seq1>&);
#if __cpp_lib_three_way_comparison
template<typename _Tp1, three_way_comparable _Seq1>
- [[nodiscard]]
friend compare_three_way_result_t<_Seq1>
operator<=>(const queue<_Tp1, _Seq1>&, const queue<_Tp1, _Seq1>&);
#endif
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 71b7dc7d295..fb8905fab08 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1127,9 +1127,8 @@ namespace views::__adaptor
|| __detail::__can_ref_view<_Range>
|| __detail::__can_subrange<_Range>
constexpr auto
- operator()(_Range&& __r) const
+ operator() [[nodiscard]] (_Range&& __r) const
noexcept(_S_noexcept<_Range>())
- [[nodiscard]]
{
if constexpr (view<decay_t<_Range>>)
return std::forward<_Range>(__r);
@@ -1554,8 +1553,7 @@ namespace views::__adaptor
template<viewable_range _Range, typename _Pred>
requires __detail::__can_filter_view<_Range, _Pred>
constexpr auto
- operator()(_Range&& __r, _Pred&& __p) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r, _Pred&& __p) const
{
return filter_view(std::forward<_Range>(__r), std::forward<_Pred>(__p));
}
@@ -1932,8 +1930,7 @@ namespace views::__adaptor
template<viewable_range _Range, typename _Fp>
requires __detail::__can_transform_view<_Range, _Fp>
constexpr auto
- operator()(_Range&& __r, _Fp&& __f) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r, _Fp&& __f) const
{
return transform_view(std::forward<_Range>(__r), std::forward<_Fp>(__f));
}
@@ -2114,8 +2111,7 @@ namespace views::__adaptor
template<viewable_range _Range, typename _Tp>
requires __detail::__can_take_view<_Range, _Tp>
constexpr auto
- operator()(_Range&& __r, _Tp&& __n) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r, _Tp&& __n) const
{
return take_view(std::forward<_Range>(__r), std::forward<_Tp>(__n));
}
@@ -2242,8 +2238,7 @@ namespace views::__adaptor
template<viewable_range _Range, typename _Pred>
requires __detail::__can_take_while_view<_Range, _Pred>
constexpr auto
- operator()(_Range&& __r, _Pred&& __p) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r, _Pred&& __p) const
{
return take_while_view(std::forward<_Range>(__r), std::forward<_Pred>(__p));
}
@@ -2363,8 +2358,7 @@ namespace views::__adaptor
template<viewable_range _Range, typename _Tp>
requires __detail::__can_drop_view<_Range, _Tp>
constexpr auto
- operator()(_Range&& __r, _Tp&& __n) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r, _Tp&& __n) const
{
return drop_view(std::forward<_Range>(__r), std::forward<_Tp>(__n));
}
@@ -2452,8 +2446,7 @@ namespace views::__adaptor
template<viewable_range _Range, typename _Pred>
requires __detail::__can_drop_while_view<_Range, _Pred>
constexpr auto
- operator()(_Range&& __r, _Pred&& __p) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r, _Pred&& __p) const
{
return drop_while_view(std::forward<_Range>(__r),
std::forward<_Pred>(__p));
@@ -2815,8 +2808,7 @@ namespace views::__adaptor
template<viewable_range _Range>
requires __detail::__can_join_view<_Range>
constexpr auto
- operator()(_Range&& __r) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r) const
{
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 3474. Nesting join_views is broken because of CTAD
@@ -3267,8 +3259,7 @@ namespace views::__adaptor
template<viewable_range _Range, typename _Pattern>
requires __detail::__can_lazy_split_view<_Range, _Pattern>
constexpr auto
- operator()(_Range&& __r, _Pattern&& __f) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r, _Pattern&& __f) const
{
return lazy_split_view(std::forward<_Range>(__r), std::forward<_Pattern>(__f));
}
@@ -3476,8 +3467,7 @@ namespace views::__adaptor
template<viewable_range _Range, typename _Pattern>
requires __detail::__can_split_view<_Range, _Pattern>
constexpr auto
- operator()(_Range&& __r, _Pattern&& __f) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r, _Pattern&& __f) const
{
return split_view(std::forward<_Range>(__r), std::forward<_Pattern>(__f));
}
@@ -3498,8 +3488,7 @@ namespace views::__adaptor
{
template<input_or_output_iterator _Iter>
constexpr auto
- operator()(_Iter __i, iter_difference_t<_Iter> __n) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Iter __i, iter_difference_t<_Iter> __n) const
{
if constexpr (random_access_iterator<_Iter>)
return subrange(__i, __i + __n);
@@ -3620,8 +3609,7 @@ namespace views::__adaptor
requires __detail::__already_common<_Range>
|| __detail::__can_common_view<_Range>
constexpr auto
- operator()(_Range&& __r) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r) const
{
if constexpr (__detail::__already_common<_Range>)
return views::all(std::forward<_Range>(__r));
@@ -3743,8 +3731,7 @@ namespace views::__adaptor
|| __detail::__is_reversible_subrange<remove_cvref_t<_Range>>
|| __detail::__can_reverse_view<_Range>
constexpr auto
- operator()(_Range&& __r) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r) const
{
using _Tp = remove_cvref_t<_Range>;
if constexpr (__detail::__is_reverse_view<_Tp>)
@@ -4142,8 +4129,7 @@ namespace views::__adaptor
template<viewable_range _Range>
requires __detail::__can_elements_view<_Nm, _Range>
constexpr auto
- operator()(_Range&& __r) const
- [[nodiscard]]
+ operator() [[nodiscard]] (_Range&& __r) const
{
return elements_view<all_t<_Range>, _Nm>{std::forward<_Range>(__r)};
}
next prev parent reply other threads:[~2021-08-05 18:02 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-04 11:55 [committed 1/2] libstdc++: Add [[nodiscard]] to iterators and related utilities Jonathan Wakely
2021-08-04 11:56 ` [committed 2/2] libstdc++: Add [[nodiscard]] to sequence containers Jonathan Wakely
2021-08-04 12:00 ` Jonathan Wakely
2021-08-05 12:09 ` Christophe Lyon
2021-08-05 12:13 ` Ville Voutilainen
2021-08-05 14:21 ` Jonathan Wakely
2021-08-05 14:38 ` Jonathan Wakely
2021-08-05 14:19 ` [committed] libstdc++: Move attributes that follow requires-clauses [PR101782] Jonathan Wakely
2021-08-05 14:27 ` Ville Voutilainen
2021-08-05 14:40 ` Jonathan Wakely
2021-08-05 18:02 ` Jonathan Wakely [this message]
2021-08-06 13:29 ` Jonathan Wakely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YQwnqPykYypmI45n@redhat.com \
--to=jwakely@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=libstdc++@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).