[libcxx-commits] [libcxx] [libc++] Makes `unique_ptr operator*() noexcept. (PR #98047)
Mark de Wever via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Jul 8 09:44:25 PDT 2024
https://github.com/mordante created https://github.com/llvm/llvm-project/pull/98047
This implements
- LWG2762 unique_ptr operator*() should be noexcept.
Differential Revision: https://reviews.llvm.org/D128214
>From 05824f10efa1c281455e75c8a49b22dd05692c67 Mon Sep 17 00:00:00 2001
From: Mark de Wever <koraq at xs4all.nl>
Date: Wed, 18 May 2022 19:56:56 +0200
Subject: [PATCH] [libc++] Makes `unique_ptr operator*() noexcept.
This implements
- LWG2762 unique_ptr operator*() should be noexcept.
Differential Revision: https://reviews.llvm.org/D128214
---
libcxx/docs/Status/Cxx23Issues.csv | 2 +-
libcxx/include/__memory/unique_ptr.h | 22 ++++++++++-
libcxx/include/memory | 3 +-
libcxx/include/optional | 25 ++++--------
.../dereference.pass.cpp | 10 +----
.../dereference_const.pass.cpp | 10 +----
.../dereference_const_rvalue.pass.cpp | 10 +----
.../dereference_rvalue.pass.cpp | 10 +----
.../optional.object.observe/op_arrow.pass.cpp | 9 +----
.../op_arrow_const.pass.cpp | 9 +----
.../dereference.single.pass.cpp | 39 ++++++++++++++++++-
11 files changed, 75 insertions(+), 74 deletions(-)
diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index cc601b3cd3c96..3e08455fcdebd 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -99,7 +99,7 @@
"","","","","",""
`2191 <https://wg21.link/LWG2191>`__,"Incorrect specification of ``match_results(match_results&&)``","October 2021","|Nothing To Do|",""
`2381 <https://wg21.link/LWG2381>`__,"Inconsistency in parsing floating point numbers","October 2021","|Complete|","19.0"
-`2762 <https://wg21.link/LWG2762>`__,"``unique_ptr operator*()`` should be ``noexcept``","October 2021","",""
+`2762 <https://wg21.link/LWG2762>`__,"``unique_ptr operator*()`` should be ``noexcept``","October 2021","|Complete|","19.0"
`3121 <https://wg21.link/LWG3121>`__,"``tuple`` constructor constraints for ``UTypes&&...`` overloads","October 2021","",""
`3123 <https://wg21.link/LWG3123>`__,"``duration`` constructor from representation shouldn't be effectively non-throwing","October 2021","","","|chrono|"
`3146 <https://wg21.link/LWG3146>`__,"Excessive unwrapping in ``std::ref/cref``","October 2021","|Complete|","14.0"
diff --git a/libcxx/include/__memory/unique_ptr.h b/libcxx/include/__memory/unique_ptr.h
index 9519e4283868b..27f6a86ca6259 100644
--- a/libcxx/include/__memory/unique_ptr.h
+++ b/libcxx/include/__memory/unique_ptr.h
@@ -36,7 +36,9 @@
#include <__type_traits/is_trivially_relocatable.h>
#include <__type_traits/is_void.h>
#include <__type_traits/remove_extent.h>
+#include <__type_traits/remove_pointer.h>
#include <__type_traits/type_identity.h>
+#include <__utility/declval.h>
#include <__utility/forward.h>
#include <__utility/move.h>
#include <cstddef>
@@ -50,6 +52,20 @@ _LIBCPP_PUSH_MACROS
_LIBCPP_BEGIN_NAMESPACE_STD
+#ifndef _LIBCPP_CXX03_LANG
+// Dereferencing _Ptr directly in noexcept fails for a void pointer.
+// This is not SFINAE-ed away leading to a hard error.
+// The issue was originally triggered by
+// test/std/utilities/memory/unique.ptr/iterator_concept_conformance.compile.pass.cpp
+template <class _Ptr>
+struct __is_noexcept_deref_or_void {
+ static constexpr bool value = noexcept(*std::declval<_Ptr>());
+};
+
+template <>
+struct __is_noexcept_deref_or_void<void*> : true_type {};
+#endif
+
template <class _Tp>
struct _LIBCPP_TEMPLATE_VIS default_delete {
static_assert(!is_function<_Tp>::value, "default_delete cannot be instantiated for function types");
@@ -252,7 +268,11 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr {
return *this;
}
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __add_lvalue_reference_t<_Tp> operator*() const {
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __add_lvalue_reference_t<_Tp> operator*() const
+#ifndef _LIBCPP_CXX03_LANG
+ noexcept(__is_noexcept_deref_or_void<pointer>::value)
+#endif
+ {
return *__ptr_.first();
}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer operator->() const _NOEXCEPT { return __ptr_.first(); }
diff --git a/libcxx/include/memory b/libcxx/include/memory
index a8c0264eb9eb7..77ab79e55c58e 100644
--- a/libcxx/include/memory
+++ b/libcxx/include/memory
@@ -451,7 +451,8 @@ public:
constexpr unique_ptr& operator=(nullptr_t) noexcept; // constexpr since C++23
// observers
- typename constexpr add_lvalue_reference<T>::type operator*() const; // constexpr since C++23
+ constexpr
+ add_lvalue_reference<T>::type operator*() const noexcept(see below); // constexpr since C++23
constexpr pointer operator->() const noexcept; // constexpr since C++23
constexpr pointer get() const noexcept; // constexpr since C++23
constexpr deleter_type& get_deleter() noexcept; // constexpr since C++23
diff --git a/libcxx/include/optional b/libcxx/include/optional
index e550745c17c00..2c41434402a65 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -136,12 +136,12 @@ namespace std {
void swap(optional &) noexcept(see below ); // constexpr in C++20
// [optional.observe], observers
- constexpr T const *operator->() const;
- constexpr T *operator->();
- constexpr T const &operator*() const &;
- constexpr T &operator*() &;
- constexpr T &&operator*() &&;
- constexpr const T &&operator*() const &&;
+ constexpr T const *operator->() const noexcept;
+ constexpr T *operator->() noexcept;
+ constexpr T const &operator*() const & noexcept;
+ constexpr T &operator*() & noexcept;
+ constexpr T &&operator*() && noexcept;
+ constexpr const T &&operator*() const && noexcept;
constexpr explicit operator bool() const noexcept;
constexpr bool has_value() const noexcept;
constexpr T const &value() const &;
@@ -783,12 +783,12 @@ public:
}
}
- _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<value_type const> operator->() const {
+ _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<value_type const> operator->() const noexcept {
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
return std::addressof(this->__get());
}
- _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<value_type> operator->() {
+ _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<value_type> operator->() noexcept {
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
return std::addressof(this->__get());
}
@@ -815,15 +815,6 @@ public:
_LIBCPP_HIDE_FROM_ABI constexpr explicit operator bool() const noexcept { return has_value(); }
- using __base::__get;
- using __base::has_value;
-
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_THROW_BAD_OPTIONAL_ACCESS constexpr value_type const& value() const& {
- if (!this->has_value())
- __throw_bad_optional_access();
- return this->__get();
- }
-
_LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_THROW_BAD_OPTIONAL_ACCESS constexpr value_type& value() & {
if (!this->has_value())
__throw_bad_optional_access();
diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference.pass.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference.pass.cpp
index 5b04e5a35aafa..49b4d21a28066 100644
--- a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference.pass.cpp
@@ -44,15 +44,7 @@ int main(int, char**)
{
optional<X> opt; ((void)opt);
ASSERT_SAME_TYPE(decltype(*opt), X&);
- LIBCPP_STATIC_ASSERT(noexcept(*opt));
- // ASSERT_NOT_NOEXCEPT(*opt);
- // FIXME: This assertion fails with GCC because it can see that
- // (A) operator*() is constexpr, and
- // (B) there is no path through the function that throws.
- // It's arguable if this is the correct behavior for the noexcept
- // operator.
- // Regardless this function should still be noexcept(false) because
- // it has a narrow contract.
+ ASSERT_NOEXCEPT(*opt);
}
{
optional<X> opt(X{});
diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_const.pass.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_const.pass.cpp
index f323cd1a5e405..ff86d9534faf6 100644
--- a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_const.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_const.pass.cpp
@@ -37,15 +37,7 @@ int main(int, char**)
{
const optional<X> opt; ((void)opt);
ASSERT_SAME_TYPE(decltype(*opt), X const&);
- LIBCPP_STATIC_ASSERT(noexcept(*opt));
- // ASSERT_NOT_NOEXCEPT(*opt);
- // FIXME: This assertion fails with GCC because it can see that
- // (A) operator*() is constexpr, and
- // (B) there is no path through the function that throws.
- // It's arguable if this is the correct behavior for the noexcept
- // operator.
- // Regardless this function should still be noexcept(false) because
- // it has a narrow contract.
+ ASSERT_NOEXCEPT(*opt);
}
{
constexpr optional<X> opt(X{});
diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_const_rvalue.pass.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_const_rvalue.pass.cpp
index 68591c5e2dbcb..646857fdc0465 100644
--- a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_const_rvalue.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_const_rvalue.pass.cpp
@@ -37,15 +37,7 @@ int main(int, char**)
{
const optional<X> opt; ((void)opt);
ASSERT_SAME_TYPE(decltype(*std::move(opt)), X const &&);
- LIBCPP_STATIC_ASSERT(noexcept(*opt));
- // ASSERT_NOT_NOEXCEPT(*std::move(opt));
- // FIXME: This assertion fails with GCC because it can see that
- // (A) operator*() is constexpr, and
- // (B) there is no path through the function that throws.
- // It's arguable if this is the correct behavior for the noexcept
- // operator.
- // Regardless this function should still be noexcept(false) because
- // it has a narrow contract.
+ ASSERT_NOEXCEPT(*std::move(opt));
}
{
constexpr optional<X> opt(X{});
diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_rvalue.pass.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_rvalue.pass.cpp
index 67edbb903353e..16bf2e4336c69 100644
--- a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_rvalue.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/dereference_rvalue.pass.cpp
@@ -44,15 +44,7 @@ int main(int, char**)
{
optional<X> opt; ((void)opt);
ASSERT_SAME_TYPE(decltype(*std::move(opt)), X&&);
- LIBCPP_STATIC_ASSERT(noexcept(*opt));
- // ASSERT_NOT_NOEXCEPT(*std::move(opt));
- // FIXME: This assertion fails with GCC because it can see that
- // (A) operator*() is constexpr, and
- // (B) there is no path through the function that throws.
- // It's arguable if this is the correct behavior for the noexcept
- // operator.
- // Regardless this function should still be noexcept(false) because
- // it has a narrow contract.
+ ASSERT_NOEXCEPT(*std::move(opt));
}
{
optional<X> opt(X{});
diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/op_arrow.pass.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/op_arrow.pass.cpp
index 7cf043f9375c1..2b5fba546ef42 100644
--- a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/op_arrow.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/op_arrow.pass.cpp
@@ -41,14 +41,7 @@ int main(int, char**)
{
std::optional<X> opt; ((void)opt);
ASSERT_SAME_TYPE(decltype(opt.operator->()), X*);
- // ASSERT_NOT_NOEXCEPT(opt.operator->());
- // FIXME: This assertion fails with GCC because it can see that
- // (A) operator->() is constexpr, and
- // (B) there is no path through the function that throws.
- // It's arguable if this is the correct behavior for the noexcept
- // operator.
- // Regardless this function should still be noexcept(false) because
- // it has a narrow contract.
+ ASSERT_NOEXCEPT(opt.operator->());
}
{
optional<X> opt(X{});
diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/op_arrow_const.pass.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/op_arrow_const.pass.cpp
index b9c1fe7794b66..d8ce932bd7810 100644
--- a/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/op_arrow_const.pass.cpp
+++ b/libcxx/test/std/utilities/optional/optional.object/optional.object.observe/op_arrow_const.pass.cpp
@@ -40,14 +40,7 @@ int main(int, char**)
{
const std::optional<X> opt; ((void)opt);
ASSERT_SAME_TYPE(decltype(opt.operator->()), X const*);
- // ASSERT_NOT_NOEXCEPT(opt.operator->());
- // FIXME: This assertion fails with GCC because it can see that
- // (A) operator->() is constexpr, and
- // (B) there is no path through the function that throws.
- // It's arguable if this is the correct behavior for the noexcept
- // operator.
- // Regardless this function should still be noexcept(false) because
- // it has a narrow contract.
+ ASSERT_NOEXCEPT(opt.operator->());
}
{
constexpr optional<X> opt(X{});
diff --git a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/dereference.single.pass.cpp b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/dereference.single.pass.cpp
index 4a1e9704afc9c..e8e267fa09b49 100644
--- a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/dereference.single.pass.cpp
+++ b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/dereference.single.pass.cpp
@@ -14,12 +14,47 @@
#include <memory>
#include <cassert>
+#include <vector>
#include "test_macros.h"
+#if TEST_STD_VER >= 11
+struct ThrowDereference {
+ TEST_CONSTEXPR_CXX23 ThrowDereference& operator*() noexcept(false);
+ TEST_CONSTEXPR_CXX23 operator bool() const { return false; }
+};
+
+struct Deleter {
+ using pointer = ThrowDereference;
+ TEST_CONSTEXPR_CXX23 void operator()(ThrowDereference&) const {}
+};
+#endif
+
TEST_CONSTEXPR_CXX23 bool test() {
- std::unique_ptr<int> p(new int(3));
- assert(*p == 3);
+ ASSERT_NOEXCEPT(*(std::unique_ptr<void>{}));
+ {
+ std::unique_ptr<int> p(new int(3));
+ assert(*p == 3);
+ ASSERT_NOEXCEPT(*p);
+ }
+#if TEST_STD_VER >= 11
+ {
+ std::unique_ptr<std::vector<int>> p(new std::vector<int>{3, 4, 5});
+ assert((*p)[0] == 3);
+ assert((*p)[1] == 4);
+ assert((*p)[2] == 5);
+ ASSERT_NOEXCEPT(*p);
+ }
+ {
+ std::unique_ptr<ThrowDereference> p;
+ ASSERT_NOEXCEPT(*p);
+ }
+ {
+ // The noexcept status of *unique_ptr<>::pointer should be propagated.
+ std::unique_ptr<ThrowDereference, Deleter> p;
+ ASSERT_NOT_NOEXCEPT(*p);
+ }
+#endif
return true;
}
More information about the libcxx-commits
mailing list