[libcxx-commits] [libcxx] [libc++] Makes `unique_ptr operator*() noexcept. (PR #98047)
Mark de Wever via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Jul 20 09:39:09 PDT 2024
https://github.com/mordante updated https://github.com/llvm/llvm-project/pull/98047
>From 712a4167003b5cb2991c2b88eb73fde3b2b01ed0 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 1/2] [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 | 16 ++++----
.../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(+), 65 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..f9cbcbfa595d1 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());
}
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;
}
>From 3f401f22d3a46520999dcf50dac54d266e6ef8c5 Mon Sep 17 00:00:00 2001
From: Mark de Wever <koraq at xs4all.nl>
Date: Sat, 20 Jul 2024 18:22:07 +0200
Subject: [PATCH 2/2] Address review comments.
---
libcxx/include/__memory/unique_ptr.h | 10 ++------
.../noexcept_operator_star.compile.pass.cpp | 25 +++++++++++++++++++
.../dereference.single.pass.cpp | 1 +
3 files changed, 28 insertions(+), 8 deletions(-)
create mode 100644 libcxx/test/std/utilities/memory/unique.ptr/noexcept_operator_star.compile.pass.cpp
diff --git a/libcxx/include/__memory/unique_ptr.h b/libcxx/include/__memory/unique_ptr.h
index 27f6a86ca6259..f75259473efb1 100644
--- a/libcxx/include/__memory/unique_ptr.h
+++ b/libcxx/include/__memory/unique_ptr.h
@@ -53,10 +53,7 @@ _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>());
@@ -269,10 +266,7 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr {
}
_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
- {
+ _NOEXCEPT_(__is_noexcept_deref_or_void<pointer>::value) {
return *__ptr_.first();
}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer operator->() const _NOEXCEPT { return __ptr_.first(); }
diff --git a/libcxx/test/std/utilities/memory/unique.ptr/noexcept_operator_star.compile.pass.cpp b/libcxx/test/std/utilities/memory/unique.ptr/noexcept_operator_star.compile.pass.cpp
new file mode 100644
index 0000000000000..a2d788005f0cd
--- /dev/null
+++ b/libcxx/test/std/utilities/memory/unique.ptr/noexcept_operator_star.compile.pass.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// unique_ptr
+
+// add_lvalue_reference_t<T> operator*() const noexcept(noexcept(*declval<pointer>()));
+
+// Dereferencing pointer 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
+//
+// This test validates whether the code compiles.
+
+#include <memory>
+
+extern const std::unique_ptr<void> p;
+void f() { [[maybe_unused]] bool b = noexcept(p.operator*()); }
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 e8e267fa09b49..a11b0692c1067 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
@@ -32,6 +32,7 @@ struct Deleter {
TEST_CONSTEXPR_CXX23 bool test() {
ASSERT_NOEXCEPT(*(std::unique_ptr<void>{}));
+ static_assert(noexcept(*std::declval<std::unique_ptr<void>>()), "");
{
std::unique_ptr<int> p(new int(3));
assert(*p == 3);
More information about the libcxx-commits
mailing list