-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[libc++][memory] Applied [[nodiscard]] to smart pointers
#168483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libc++][memory] Applied [[nodiscard]] to smart pointers
#168483
Conversation
|
@llvm/pr-subscribers-libcxx Author: Hristo Hristov (H-G-Hristov) ChangesApplied
See guidelines:
Patch is 28.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/168483.diff 3 Files Affected:
diff --git a/libcxx/include/__memory/shared_ptr.h b/libcxx/include/__memory/shared_ptr.h
index ee07efe081e51..76ae50db20b6d 100644
--- a/libcxx/include/__memory/shared_ptr.h
+++ b/libcxx/include/__memory/shared_ptr.h
@@ -568,16 +568,20 @@ class _LIBCPP_SHARED_PTR_TRIVIAL_ABI shared_ptr {
shared_ptr(__p, __d, __a).swap(*this);
}
- _LIBCPP_HIDE_FROM_ABI element_type* get() const _NOEXCEPT { return __ptr_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI element_type* get() const _NOEXCEPT { return __ptr_; }
- _LIBCPP_HIDE_FROM_ABI __add_lvalue_reference_t<element_type> operator*() const _NOEXCEPT { return *__ptr_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI __add_lvalue_reference_t<element_type> operator*() const _NOEXCEPT {
+ return *__ptr_;
+ }
_LIBCPP_HIDE_FROM_ABI element_type* operator->() const _NOEXCEPT {
static_assert(!is_array<_Tp>::value, "std::shared_ptr<T>::operator-> is only valid when T is not an array type.");
return __ptr_;
}
- _LIBCPP_HIDE_FROM_ABI long use_count() const _NOEXCEPT { return __cntrl_ ? __cntrl_->use_count() : 0; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI long use_count() const _NOEXCEPT {
+ return __cntrl_ ? __cntrl_->use_count() : 0;
+ }
#if _LIBCPP_STD_VER < 20 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_SHARED_PTR_UNIQUE)
_LIBCPP_DEPRECATED_IN_CXX17 _LIBCPP_HIDE_FROM_ABI bool unique() const _NOEXCEPT { return use_count() == 1; }
@@ -586,19 +590,19 @@ class _LIBCPP_SHARED_PTR_TRIVIAL_ABI shared_ptr {
_LIBCPP_HIDE_FROM_ABI explicit operator bool() const _NOEXCEPT { return get() != nullptr; }
template <class _Up>
- _LIBCPP_HIDE_FROM_ABI bool owner_before(shared_ptr<_Up> const& __p) const _NOEXCEPT {
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool owner_before(shared_ptr<_Up> const& __p) const _NOEXCEPT {
return __cntrl_ < __p.__cntrl_;
}
template <class _Up>
- _LIBCPP_HIDE_FROM_ABI bool owner_before(weak_ptr<_Up> const& __p) const _NOEXCEPT {
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool owner_before(weak_ptr<_Up> const& __p) const _NOEXCEPT {
return __cntrl_ < __p.__cntrl_;
}
_LIBCPP_HIDE_FROM_ABI bool __owner_equivalent(const shared_ptr& __p) const { return __cntrl_ == __p.__cntrl_; }
#if _LIBCPP_STD_VER >= 17
- _LIBCPP_HIDE_FROM_ABI __add_lvalue_reference_t<element_type> operator[](ptrdiff_t __i) const {
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI __add_lvalue_reference_t<element_type> operator[](ptrdiff_t __i) const {
static_assert(is_array<_Tp>::value, "std::shared_ptr<T>::operator[] is only valid when T is an array type.");
return __ptr_[__i];
}
@@ -669,7 +673,7 @@ shared_ptr(unique_ptr<_Tp, _Dp>) -> shared_ptr<_Tp>;
// std::allocate_shared and std::make_shared
//
template <class _Tp, class _Alloc, class... _Args, __enable_if_t<!is_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared(const _Alloc& __a, _Args&&... __args) {
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared(const _Alloc& __a, _Args&&... __args) {
using _ControlBlock = __shared_ptr_emplace<_Tp, _Alloc>;
using _ControlBlockAllocator = typename __allocator_traits_rebind<_Alloc, _ControlBlock>::type;
__allocation_guard<_ControlBlockAllocator> __guard(__a, 1);
@@ -680,21 +684,21 @@ _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared(const _Alloc& __a, _Args&&
}
template <class _Tp, class... _Args, __enable_if_t<!is_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared(_Args&&... __args) {
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared(_Args&&... __args) {
return std::allocate_shared<_Tp>(allocator<__remove_cv_t<_Tp> >(), std::forward<_Args>(__args)...);
}
#if _LIBCPP_STD_VER >= 20
template <class _Tp, class _Alloc, __enable_if_t<!is_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared_for_overwrite(const _Alloc& __a) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared_for_overwrite(const _Alloc& __a) {
using _ForOverwriteAllocator = __allocator_traits_rebind_t<_Alloc, __for_overwrite_tag>;
_ForOverwriteAllocator __alloc(__a);
return std::allocate_shared<_Tp>(__alloc);
}
template <class _Tp, __enable_if_t<!is_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared_for_overwrite() {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared_for_overwrite() {
return std::allocate_shared_for_overwrite<_Tp>(allocator<__remove_cv_t<_Tp>>());
}
@@ -886,67 +890,69 @@ _LIBCPP_HIDE_FROM_ABI shared_ptr<_Array> __allocate_shared_bounded_array(const _
// bounded array variants
template <class _Tp, class _Alloc, __enable_if_t<is_bounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared(const _Alloc& __a) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared(const _Alloc& __a) {
return std::__allocate_shared_bounded_array<_Tp>(__a);
}
template <class _Tp, class _Alloc, __enable_if_t<is_bounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared(const _Alloc& __a, const remove_extent_t<_Tp>& __u) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp>
+allocate_shared(const _Alloc& __a, const remove_extent_t<_Tp>& __u) {
return std::__allocate_shared_bounded_array<_Tp>(__a, __u);
}
template <class _Tp, class _Alloc, __enable_if_t<is_bounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared_for_overwrite(const _Alloc& __a) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared_for_overwrite(const _Alloc& __a) {
using _ForOverwriteAllocator = __allocator_traits_rebind_t<_Alloc, __for_overwrite_tag>;
_ForOverwriteAllocator __alloc(__a);
return std::__allocate_shared_bounded_array<_Tp>(__alloc);
}
template <class _Tp, __enable_if_t<is_bounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared() {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared() {
return std::__allocate_shared_bounded_array<_Tp>(allocator<_Tp>());
}
template <class _Tp, __enable_if_t<is_bounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared(const remove_extent_t<_Tp>& __u) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared(const remove_extent_t<_Tp>& __u) {
return std::__allocate_shared_bounded_array<_Tp>(allocator<_Tp>(), __u);
}
template <class _Tp, __enable_if_t<is_bounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared_for_overwrite() {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared_for_overwrite() {
return std::__allocate_shared_bounded_array<_Tp>(allocator<__for_overwrite_tag>());
}
// unbounded array variants
template <class _Tp, class _Alloc, __enable_if_t<is_unbounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared(const _Alloc& __a, size_t __n) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared(const _Alloc& __a, size_t __n) {
return std::__allocate_shared_unbounded_array<_Tp>(__a, __n);
}
template <class _Tp, class _Alloc, __enable_if_t<is_unbounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared(const _Alloc& __a, size_t __n, const remove_extent_t<_Tp>& __u) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp>
+allocate_shared(const _Alloc& __a, size_t __n, const remove_extent_t<_Tp>& __u) {
return std::__allocate_shared_unbounded_array<_Tp>(__a, __n, __u);
}
template <class _Tp, class _Alloc, __enable_if_t<is_unbounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared_for_overwrite(const _Alloc& __a, size_t __n) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> allocate_shared_for_overwrite(const _Alloc& __a, size_t __n) {
using _ForOverwriteAllocator = __allocator_traits_rebind_t<_Alloc, __for_overwrite_tag>;
_ForOverwriteAllocator __alloc(__a);
return std::__allocate_shared_unbounded_array<_Tp>(__alloc, __n);
}
template <class _Tp, __enable_if_t<is_unbounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared(size_t __n) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared(size_t __n) {
return std::__allocate_shared_unbounded_array<_Tp>(allocator<_Tp>(), __n);
}
template <class _Tp, __enable_if_t<is_unbounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared(size_t __n, const remove_extent_t<_Tp>& __u) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared(size_t __n, const remove_extent_t<_Tp>& __u) {
return std::__allocate_shared_unbounded_array<_Tp>(allocator<_Tp>(), __n, __u);
}
template <class _Tp, __enable_if_t<is_unbounded_array<_Tp>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared_for_overwrite(size_t __n) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> make_shared_for_overwrite(size_t __n) {
return std::__allocate_shared_unbounded_array<_Tp>(allocator<__for_overwrite_tag>(), __n);
}
@@ -980,12 +986,14 @@ inline _LIBCPP_HIDE_FROM_ABI bool operator>(const shared_ptr<_Tp>& __x, const sh
}
template <class _Tp, class _Up>
-inline _LIBCPP_HIDE_FROM_ABI bool operator<=(const shared_ptr<_Tp>& __x, const shared_ptr<_Up>& __y) _NOEXCEPT {
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI bool
+operator<=(const shared_ptr<_Tp>& __x, const shared_ptr<_Up>& __y) _NOEXCEPT {
return !(__y < __x);
}
template <class _Tp, class _Up>
-inline _LIBCPP_HIDE_FROM_ABI bool operator>=(const shared_ptr<_Tp>& __x, const shared_ptr<_Up>& __y) _NOEXCEPT {
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI bool
+operator>=(const shared_ptr<_Tp>& __x, const shared_ptr<_Up>& __y) _NOEXCEPT {
return !(__x < __y);
}
@@ -1075,7 +1083,8 @@ inline _LIBCPP_HIDE_FROM_ABI void swap(shared_ptr<_Tp>& __x, shared_ptr<_Tp>& __
}
template <class _Tp, class _Up>
-inline _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> static_pointer_cast(const shared_ptr<_Up>& __r) _NOEXCEPT {
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp>
+static_pointer_cast(const shared_ptr<_Up>& __r) _NOEXCEPT {
return shared_ptr<_Tp>(__r, static_cast< typename shared_ptr<_Tp>::element_type*>(__r.get()));
}
@@ -1083,13 +1092,14 @@ inline _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> static_pointer_cast(const shared_pt
// We don't backport because it is an evolutionary change.
#if _LIBCPP_STD_VER >= 20
template <class _Tp, class _Up>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> static_pointer_cast(shared_ptr<_Up>&& __r) noexcept {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> static_pointer_cast(shared_ptr<_Up>&& __r) noexcept {
return shared_ptr<_Tp>(std::move(__r), static_cast<typename shared_ptr<_Tp>::element_type*>(__r.get()));
}
#endif
template <class _Tp, class _Up>
-inline _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> dynamic_pointer_cast(const shared_ptr<_Up>& __r) _NOEXCEPT {
+[[__nodiscard__]] inline
+ _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> dynamic_pointer_cast(const shared_ptr<_Up>& __r) _NOEXCEPT {
typedef typename shared_ptr<_Tp>::element_type _ET;
_ET* __p = dynamic_cast<_ET*>(__r.get());
return __p ? shared_ptr<_Tp>(__r, __p) : shared_ptr<_Tp>();
@@ -1099,14 +1109,14 @@ inline _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> dynamic_pointer_cast(const shared_p
// We don't backport because it is an evolutionary change.
#if _LIBCPP_STD_VER >= 20
template <class _Tp, class _Up>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> dynamic_pointer_cast(shared_ptr<_Up>&& __r) noexcept {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> dynamic_pointer_cast(shared_ptr<_Up>&& __r) noexcept {
auto* __p = dynamic_cast<typename shared_ptr<_Tp>::element_type*>(__r.get());
return __p ? shared_ptr<_Tp>(std::move(__r), __p) : shared_ptr<_Tp>();
}
#endif
template <class _Tp, class _Up>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> const_pointer_cast(const shared_ptr<_Up>& __r) _NOEXCEPT {
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> const_pointer_cast(const shared_ptr<_Up>& __r) _NOEXCEPT {
typedef typename shared_ptr<_Tp>::element_type _RTp;
return shared_ptr<_Tp>(__r, const_cast<_RTp*>(__r.get()));
}
@@ -1115,13 +1125,13 @@ _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> const_pointer_cast(const shared_ptr<_Up>&
// We don't backport because it is an evolutionary change.
#if _LIBCPP_STD_VER >= 20
template <class _Tp, class _Up>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> const_pointer_cast(shared_ptr<_Up>&& __r) noexcept {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> const_pointer_cast(shared_ptr<_Up>&& __r) noexcept {
return shared_ptr<_Tp>(std::move(__r), const_cast<typename shared_ptr<_Tp>::element_type*>(__r.get()));
}
#endif
template <class _Tp, class _Up>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> reinterpret_pointer_cast(const shared_ptr<_Up>& __r) _NOEXCEPT {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> reinterpret_pointer_cast(const shared_ptr<_Up>& __r) _NOEXCEPT {
return shared_ptr<_Tp>(__r, reinterpret_cast< typename shared_ptr<_Tp>::element_type*>(__r.get()));
}
@@ -1129,7 +1139,7 @@ _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> reinterpret_pointer_cast(const shared_ptr<
// We don't backport because it is an evolutionary change.
#if _LIBCPP_STD_VER >= 20
template <class _Tp, class _Up>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> reinterpret_pointer_cast(shared_ptr<_Up>&& __r) noexcept {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> reinterpret_pointer_cast(shared_ptr<_Up>&& __r) noexcept {
return shared_ptr<_Tp>(std::move(__r), reinterpret_cast<typename shared_ptr<_Tp>::element_type*>(__r.get()));
}
#endif
@@ -1137,7 +1147,7 @@ _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> reinterpret_pointer_cast(shared_ptr<_Up>&&
#if _LIBCPP_HAS_RTTI
template <class _Dp, class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI _Dp* get_deleter(const shared_ptr<_Tp>& __p) _NOEXCEPT {
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _Dp* get_deleter(const shared_ptr<_Tp>& __p) _NOEXCEPT {
return __p.template __get_deleter<_Dp>();
}
@@ -1192,15 +1202,19 @@ class _LIBCPP_SHARED_PTR_TRIVIAL_ABI weak_ptr {
_LIBCPP_HIDE_FROM_ABI void swap(weak_ptr& __r) _NOEXCEPT;
_LIBCPP_HIDE_FROM_ABI void reset() _NOEXCEPT;
- _LIBCPP_HIDE_FROM_ABI long use_count() const _NOEXCEPT { return __cntrl_ ? __cntrl_->use_count() : 0; }
- _LIBCPP_HIDE_FROM_ABI bool expired() const _NOEXCEPT { return __cntrl_ == nullptr || __cntrl_->use_count() == 0; }
- _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> lock() const _NOEXCEPT;
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI long use_count() const _NOEXCEPT {
+ return __cntrl_ ? __cntrl_->use_count() : 0;
+ }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool expired() const _NOEXCEPT {
+ return __cntrl_ == nullptr || __cntrl_->use_count() == 0;
+ }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> lock() const _NOEXCEPT;
template <class _Up>
- _LIBCPP_HIDE_FROM_ABI bool owner_before(const shared_ptr<_Up>& __r) const _NOEXCEPT {
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool owner_before(const shared_ptr<_Up>& __r) const _NOEXCEPT {
return __cntrl_ < __r.__cntrl_;
}
template <class _Up>
- _LIBCPP_HIDE_FROM_ABI bool owner_before(const weak_ptr<_Up>& __r) const _NOEXCEPT {
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool owner_before(const weak_ptr<_Up>& __r) const _NOEXCEPT {
return __cntrl_ < __r.__cntrl_;
}
@@ -1384,13 +1398,15 @@ class enable_shared_from_this {
_LIBCPP_HIDE_FROM_ABI ~enable_shared_from_this() {}
public:
- _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> shared_from_this() { return shared_ptr<_Tp>(__weak_this_); }
- _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp const> shared_from_this() const { return shared_ptr<const _Tp>(__weak_this_); }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> shared_from_this() { return shared_ptr<_Tp>(__weak_this_); }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp const> shared_from_this() const {
+ return shared_ptr<const _Tp>(__weak_this_);
+ }
#if _LIBCPP_STD_VER >= 17
- _LIBCPP_HIDE_FROM_ABI weak_ptr<_Tp> weak_from_this() _NOEXCEPT { return __weak_this_; }
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI weak_ptr<_Tp> weak_from_this() _NOEXCEPT { return __weak_this_; }
- _LIBCPP_HIDE_FROM_ABI weak_ptr<const _Tp> weak_from_this() const _NOEXCEPT { return __weak_this_; }
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI weak_ptr<const _Tp> weak_from_this() const _NOEXCEPT { return __weak_this_; }
#endif // _LIBCPP_STD_VER >= 17
template <class _Up>
diff --git a/libcxx/include/__memory/unique_ptr.h b/libcxx/include/__memory/unique_ptr.h
index 3cf4b97a7f49c..a822c761be203 100644
--- a/libcxx/include/__memory/unique_ptr.h
+++ b/libcxx/include/__memory/unique_ptr.h
@@ -258,14 +258,17 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr {
return *this;
}
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __add_lvalue_reference_t<_Tp> operator*() const
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __add_lvalue_reference_t<_Tp> operator*() const
_NOEXCEPT_(_NOEXCEPT_(*std::declval<pointer>())) {
return *__ptr_;
}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer operator->() const _NOEXCEPT { return __ptr_; }
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer get() const _NOEXCEPT { return __ptr_; }
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 deleter_type& get_deleter() _NOEXCEPT { return __deleter_; }
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 const deleter_type& get_deleter() const _NOEXCEPT {
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer get() const _NOEXCEPT { return __ptr_; }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 deleter_type& get_deleter() _NOEXCEPT {
+ return __deleter_;
+ }
+ [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 const deleter_type&
+ get_deleter() const _NOEXCEPT {
return __deleter_;
}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 explicit operator bool() const _NOEXCEPT {
@@ -672,8 +675,8 @@ operator<=>(const unique_ptr<_T1, _D1>& __x, const unique_ptr<_T2, _D2>& __y) {
#endif
template <class _T1, class _D1>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool
-operator==(const unique_ptr<_T1, _D1>& __x, nullptr_t) _NOEXCEPT {
+inline _LIBCPP_HIDE_FROM_ABI
+_LIBCPP_CONSTEXPR_SINCE_CXX23 bool operator==(const unique_ptr<_T1, _D1>& __x, nullptr_t) _NOEXCEPT {
return !__x;
}
@@ -748,12 +751,13 @@ operator<=>(const unique_ptr<_T1, _D1>& __x, nullptr_t) {
#if _LIBCPP_STD_VER >= 14
template <class _Tp, class... _Args, enable_if_t<!is_array<_Tp>::value, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr<_Tp> make_unique(_Args&&... __args) {
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI
+_LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr<_Tp> make_unique(_Args&&... __args) {
return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...));
}
template <class _Tp, enable_if_t<__is_unbounded_array_v<_Tp>, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr<_Tp> make_unique(size_t __n) {
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr<_Tp> make_unique(size_t __n) {
typedef __remove_extent_t<_Tp> _Up;
return unique_ptr<_Tp>(__private_constructor_tag(), new _Up[__n](), __n);
}
@@ -766,12 +770,13 @@ void make_unique(_Args&&...) = delete;
#if _LIBCPP_STD_VER >= 20
template <class _Tp, enable_if_t<!is_array_v<_Tp>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr<_Tp> make_unique_for_overwrite() {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr<_Tp> make_unique_for_overwrite() {
return unique_ptr<_Tp>(new _Tp);
}
template <class _Tp, enable_if_t<is_unbounded_array_v<_Tp>, int> = 0>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr<_Tp> make_unique_for_overwrite(size_t __n) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr<_Tp>
+make_unique_for_overwrite(size_t __n) {
return unique_ptr<_Tp>(__private_constructor_tag(), new __remove_extent_t<_Tp>[__n], __n);
}
diff --git a/libcxx/test/libcxx/utilities/smartptr/nodiscard.verify.cpp b/libcxx/test/libcxx/utilities/smartp...
[truncated]
|
a7f9ee6 to
b2266e0
Compare
Applied `[[nodiscard]]` where relevant to smart pointers and related functions. See guidelines: - https://libcxx.llvm.org/CodingGuidelines.html#apply-nodiscard-where-relevant - `[[nodiscard]]` should be applied to functions where discarding the return value is most likely a correctness issue. For example a locking constructor in unique_lock.
b2266e0 to
ff03257
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
philnik777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % nit.
...smartptr/util.smartptr.shared/util.smartptr.shared.obs/unique.deprecated_in_cxx17.verify.cpp
Outdated
Show resolved
Hide resolved
Thank you! |
|
Thanks for all the For It's a pretty small N, so I don't suggest doing anything in response to this data point, but if other projects are seeing this too, maybe it might make sense to roll back the make_unique nodiscard. I can easily imagine it catching bugs. But maybe creating objects for side effects is common enough that it isn't worth it in total. (Or maybe it is! Just contributing a data point.) |
That good to hear! Do you have any statistics on this? It'd be really interesting to see what kind of bugs this find.
3/4 were death tests where I'm guessing using |
Recent versions of libc++ mark make_unique<>'s result as [[nodiscard]]: llvm/llvm-project#168483 These places all intentionally return make_unique<>'s result, so explicitly annotate them as such. Bug: none Change-Id: I32077a72c7f343d77b70ba22a9f6ba23f4384a58 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7199487 Reviewed-by: Hans Wennborg <[email protected]> Commit-Queue: Nico Weber <[email protected]> Owners-Override: Nico Weber <[email protected]> Cr-Commit-Position: refs/heads/main@{#1549979}
Applied `[[nodiscard]]` where relevant to smart pointers and related functions. - [x] - `std::unique_ptr` - [x] - `std::shared_ptr` - [x] - `std::weak_ptr` See guidelines: - https://libcxx.llvm.org/CodingGuidelines.html#apply-nodiscard-where-relevant - `[[nodiscard]]` should be applied to functions where discarding the return value is most likely a correctness issue. For example a locking constructor in unique_lock. --------- Co-authored-by: Hristo Hristov <[email protected]>
Applied `[[nodiscard]]` where relevant to smart pointers and related functions. - [x] - `std::unique_ptr` - [x] - `std::shared_ptr` - [x] - `std::weak_ptr` See guidelines: - https://libcxx.llvm.org/CodingGuidelines.html#apply-nodiscard-where-relevant - `[[nodiscard]]` should be applied to functions where discarding the return value is most likely a correctness issue. For example a locking constructor in unique_lock. --------- Co-authored-by: Hristo Hristov <[email protected]>
|
(Here's a fifth; still haven't looked at the missing thing I mentioned above: https://chromium-review.googlesource.com/c/chromium/src/+/7207549 Also a false positive here; the test uses make_unique<> to force an alloc/free pair.) |
Recent versions of libc++ mark make_unique<>'s result as [[nodiscard]]: llvm/llvm-project#168483 This place intentionally returns make_unique<>'s result, so explicitly annotate it as such. Previously: https://chromium-review.googlesource.com/c/chromium/src/+/7199487 This instance here is android-only, which is why it wasn't part of that CL. No behavior change. Bug: none Change-Id: I3293f65934d34cfd63795a6b34008aa4a8d78237 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7207549 Reviewed-by: Hans Wennborg <[email protected]> Commit-Queue: Hans Wennborg <[email protected]> Auto-Submit: Nico Weber <[email protected]> Cr-Commit-Position: refs/heads/main@{#1550606}
|
Here's two more that just test that a create/destroy sequence doesn't crash (not as a death test): For the one I wanted to look into, it also looks like the make_unique<> result was intentionally not used, but the code is a bit convoluted: http://crbug.com/463698813 So for us it's 9/9 false positives. (But, again, no action needed, just contributing a data point.) |
Recent versions of libc++ mark make_unique<>'s result as [[nodiscard]]: llvm/llvm-project#168483 This place intentionally returns make_unique<>'s result, so explicitly annotate it as such. Previously: https://chromium-review.googlesource.com/c/chromium/src/+/7199487 This instance here is android-only, which is why it wasn't part of that CL. No behavior change. Bug: none Change-Id: I3293f65934d34cfd63795a6b34008aa4a8d78237 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7207549 Reviewed-by: Hans Wennborg <[email protected]> Commit-Queue: Hans Wennborg <[email protected]> Auto-Submit: Nico Weber <[email protected]> Cr-Commit-Position: refs/heads/main@{#1550606} NOKEYCHECK=True GitOrigin-RevId: 1383ff0a2548cf347e245d797e1144885da2231f
Applied
[[nodiscard]]where relevant to smart pointers and related functions.std::unique_ptrstd::shared_ptrstd::weak_ptrSee guidelines:
[[nodiscard]]should be applied to functions where discarding the return value is most likely a correctness issue. For example a locking constructor in unique_lock.