* warnings about unused shared_ptr/unique_ptr comparisons
@ 2019-01-14 15:53 Ulrich Drepper
2019-01-14 16:04 ` Kyrill Tkachov
2019-01-14 20:41 ` Jonathan Wakely
0 siblings, 2 replies; 3+ messages in thread
From: Ulrich Drepper @ 2019-01-14 15:53 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1.1.1: Type: text/plain, Size: 1291 bytes --]
This is a conservative implementation of a patch to make
shared/unique_ptrs behave more like plain old pointers. More about this
in bug #88738
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88738
The summary is
- using clang, which enables a warning for unused results of all
comparison operation, found a real bug
- a library implementation is limited in scope and tedious to add
everywhere. At this stage of gcc 9 it was the only acceptable solution,
though
- longer term there should be a warning for comparison operators.
Possibly on by default with the possibility to disable it with an
attribute (see the discussion in the bug).
The patch proposed here only changes the code for C++17 and up to use
the [[nodiscard]] attribute. For gcc 10 we can either widen this or
implement a better way with the help of the compiler.
I ran the regression test suite and didn't see any additional failures.
OK?
libstdc++-v3/
2019-02-14 Ulrich Drepper <drepper@redhat.com>
PR libstdc++/88738
Warn about unused comparisons of shared_ptr/unique_ptr
* include/bits/c++config [_GLIBCXX_NODISCARD]: Define.
* include/bits/shared_ptr.h: Use it for operator ==, !=,
<, <=, >, >= for shared_ptr.
* include/bits/unique_ptr.h: Likewise for unique_ptr.
[-- Attachment #1.1.2: d-c++-nodiscard-17 --]
[-- Type: text/plain, Size: 8806 bytes --]
diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index 9b2fabd7d76..97bb6db70b1 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -99,6 +99,14 @@
# define _GLIBCXX_ABI_TAG_CXX11 __attribute ((__abi_tag__ ("cxx11")))
#endif
+// Macro to warn about unused results.
+#if __cplusplus >= 201703L
+# define _GLIBCXX_NODISCARD [[__nodiscard__]]
+#else
+# define _GLIBCXX_NODISCARD
+#endif
+
+
#if __cplusplus
diff --git a/libstdc++-v3/include/bits/shared_ptr.h b/libstdc++-v3/include/bits/shared_ptr.h
index 99009ab4f99..d504627d1a0 100644
--- a/libstdc++-v3/include/bits/shared_ptr.h
+++ b/libstdc++-v3/include/bits/shared_ptr.h
@@ -380,37 +380,37 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// 20.7.2.2.7 shared_ptr comparisons
template<typename _Tp, typename _Up>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator==(const shared_ptr<_Tp>& __a, const shared_ptr<_Up>& __b) noexcept
{ return __a.get() == __b.get(); }
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator==(const shared_ptr<_Tp>& __a, nullptr_t) noexcept
{ return !__a; }
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator==(nullptr_t, const shared_ptr<_Tp>& __a) noexcept
{ return !__a; }
template<typename _Tp, typename _Up>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator!=(const shared_ptr<_Tp>& __a, const shared_ptr<_Up>& __b) noexcept
{ return __a.get() != __b.get(); }
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator!=(const shared_ptr<_Tp>& __a, nullptr_t) noexcept
{ return (bool)__a; }
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator!=(nullptr_t, const shared_ptr<_Tp>& __a) noexcept
{ return (bool)__a; }
template<typename _Tp, typename _Up>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<(const shared_ptr<_Tp>& __a, const shared_ptr<_Up>& __b) noexcept
{
using _Tp_elt = typename shared_ptr<_Tp>::element_type;
@@ -420,7 +420,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<(const shared_ptr<_Tp>& __a, nullptr_t) noexcept
{
using _Tp_elt = typename shared_ptr<_Tp>::element_type;
@@ -428,7 +428,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<(nullptr_t, const shared_ptr<_Tp>& __a) noexcept
{
using _Tp_elt = typename shared_ptr<_Tp>::element_type;
@@ -436,47 +436,47 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename _Tp, typename _Up>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<=(const shared_ptr<_Tp>& __a, const shared_ptr<_Up>& __b) noexcept
{ return !(__b < __a); }
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<=(const shared_ptr<_Tp>& __a, nullptr_t) noexcept
{ return !(nullptr < __a); }
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<=(nullptr_t, const shared_ptr<_Tp>& __a) noexcept
{ return !(__a < nullptr); }
template<typename _Tp, typename _Up>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>(const shared_ptr<_Tp>& __a, const shared_ptr<_Up>& __b) noexcept
{ return (__b < __a); }
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>(const shared_ptr<_Tp>& __a, nullptr_t) noexcept
{ return nullptr < __a; }
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>(nullptr_t, const shared_ptr<_Tp>& __a) noexcept
{ return __a < nullptr; }
template<typename _Tp, typename _Up>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>=(const shared_ptr<_Tp>& __a, const shared_ptr<_Up>& __b) noexcept
{ return !(__a < __b); }
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>=(const shared_ptr<_Tp>& __a, nullptr_t) noexcept
{ return !(__a < nullptr); }
template<typename _Tp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>=(nullptr_t, const shared_ptr<_Tp>& __a) noexcept
{ return !(nullptr < __a); }
diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h
index c7eb02e86fd..61a3ef05460 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -707,41 +707,41 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _Tp, typename _Dp,
typename _Up, typename _Ep>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator==(const unique_ptr<_Tp, _Dp>& __x,
const unique_ptr<_Up, _Ep>& __y)
{ return __x.get() == __y.get(); }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator==(const unique_ptr<_Tp, _Dp>& __x, nullptr_t) noexcept
{ return !__x; }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator==(nullptr_t, const unique_ptr<_Tp, _Dp>& __x) noexcept
{ return !__x; }
template<typename _Tp, typename _Dp,
typename _Up, typename _Ep>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator!=(const unique_ptr<_Tp, _Dp>& __x,
const unique_ptr<_Up, _Ep>& __y)
{ return __x.get() != __y.get(); }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator!=(const unique_ptr<_Tp, _Dp>& __x, nullptr_t) noexcept
{ return (bool)__x; }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator!=(nullptr_t, const unique_ptr<_Tp, _Dp>& __x) noexcept
{ return (bool)__x; }
template<typename _Tp, typename _Dp,
typename _Up, typename _Ep>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<(const unique_ptr<_Tp, _Dp>& __x,
const unique_ptr<_Up, _Ep>& __y)
{
@@ -752,67 +752,67 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<(const unique_ptr<_Tp, _Dp>& __x, nullptr_t)
{ return std::less<typename unique_ptr<_Tp, _Dp>::pointer>()(__x.get(),
nullptr); }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<(nullptr_t, const unique_ptr<_Tp, _Dp>& __x)
{ return std::less<typename unique_ptr<_Tp, _Dp>::pointer>()(nullptr,
__x.get()); }
template<typename _Tp, typename _Dp,
typename _Up, typename _Ep>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<=(const unique_ptr<_Tp, _Dp>& __x,
const unique_ptr<_Up, _Ep>& __y)
{ return !(__y < __x); }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<=(const unique_ptr<_Tp, _Dp>& __x, nullptr_t)
{ return !(nullptr < __x); }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator<=(nullptr_t, const unique_ptr<_Tp, _Dp>& __x)
{ return !(__x < nullptr); }
template<typename _Tp, typename _Dp,
typename _Up, typename _Ep>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>(const unique_ptr<_Tp, _Dp>& __x,
const unique_ptr<_Up, _Ep>& __y)
{ return (__y < __x); }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>(const unique_ptr<_Tp, _Dp>& __x, nullptr_t)
{ return std::less<typename unique_ptr<_Tp, _Dp>::pointer>()(nullptr,
__x.get()); }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>(nullptr_t, const unique_ptr<_Tp, _Dp>& __x)
{ return std::less<typename unique_ptr<_Tp, _Dp>::pointer>()(__x.get(),
nullptr); }
template<typename _Tp, typename _Dp,
typename _Up, typename _Ep>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>=(const unique_ptr<_Tp, _Dp>& __x,
const unique_ptr<_Up, _Ep>& __y)
{ return !(__x < __y); }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>=(const unique_ptr<_Tp, _Dp>& __x, nullptr_t)
{ return !(__x < nullptr); }
template<typename _Tp, typename _Dp>
- inline bool
+ _GLIBCXX_NODISCARD inline bool
operator>=(nullptr_t, const unique_ptr<_Tp, _Dp>& __x)
{ return !(nullptr < __x); }
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: warnings about unused shared_ptr/unique_ptr comparisons
2019-01-14 15:53 warnings about unused shared_ptr/unique_ptr comparisons Ulrich Drepper
@ 2019-01-14 16:04 ` Kyrill Tkachov
2019-01-14 20:41 ` Jonathan Wakely
1 sibling, 0 replies; 3+ messages in thread
From: Kyrill Tkachov @ 2019-01-14 16:04 UTC (permalink / raw)
To: Ulrich Drepper, gcc-patches; +Cc: libstdc++
On 14/01/19 15:53, Ulrich Drepper wrote:
> This is a conservative implementation of a patch to make
> shared/unique_ptrs behave more like plain old pointers. More about this
> in bug #88738
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88738
>
> The summary is
>
> - using clang, which enables a warning for unused results of all
> comparison operation, found a real bug
>
> - a library implementation is limited in scope and tedious to add
> everywhere. At this stage of gcc 9 it was the only acceptable solution,
> though
>
> - longer term there should be a warning for comparison operators.
> Possibly on by default with the possibility to disable it with an
> attribute (see the discussion in the bug).
>
>
> The patch proposed here only changes the code for C++17 and up to use
> the [[nodiscard]] attribute. For gcc 10 we can either widen this or
> implement a better way with the help of the compiler.
>
> I ran the regression test suite and didn't see any additional failures.
>
> OK?
Forwarding to the libstdc++ list for these patches.
Thanks,
Kyrill
> libstdc++-v3/
> 2019-02-14 Ulrich Drepper <drepper@redhat.com>
>
> PR libstdc++/88738
> Warn about unused comparisons of shared_ptr/unique_ptr
> * include/bits/c++config [_GLIBCXX_NODISCARD]: Define.
> * include/bits/shared_ptr.h: Use it for operator ==, !=,
> <, <=, >, >= for shared_ptr.
> * include/bits/unique_ptr.h: Likewise for unique_ptr.
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: warnings about unused shared_ptr/unique_ptr comparisons
2019-01-14 15:53 warnings about unused shared_ptr/unique_ptr comparisons Ulrich Drepper
2019-01-14 16:04 ` Kyrill Tkachov
@ 2019-01-14 20:41 ` Jonathan Wakely
1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2019-01-14 20:41 UTC (permalink / raw)
To: Ulrich Drepper; +Cc: gcc-patches, libstdc++
On 14/01/19 16:53 +0100, Ulrich Drepper wrote:
>This is a conservative implementation of a patch to make
>shared/unique_ptrs behave more like plain old pointers. More about this
>in bug #88738
>
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88738
>
>The summary is
>
>- using clang, which enables a warning for unused results of all
>comparison operation, found a real bug
>
>- a library implementation is limited in scope and tedious to add
>everywhere. At this stage of gcc 9 it was the only acceptable solution,
>though
>
>- longer term there should be a warning for comparison operators.
>Possibly on by default with the possibility to disable it with an
>attribute (see the discussion in the bug).
>
>
>The patch proposed here only changes the code for C++17 and up to use
>the [[nodiscard]] attribute. For gcc 10 we can either widen this or
>implement a better way with the help of the compiler.
>
>I ran the regression test suite and didn't see any additional failures.
>
>OK?
As it only makes changes for C++17 and up, this is OK for trunk now.
Thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-14 20:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-14 15:53 warnings about unused shared_ptr/unique_ptr comparisons Ulrich Drepper
2019-01-14 16:04 ` Kyrill Tkachov
2019-01-14 20:41 ` 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).