[libc++] Makes `unique_ptr operator*() noexcept. #98047
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mordante
force-pushed
the
phabricator/D128214-LWG2762
branch
from
05824f1 to
6c3f8e6
Compare
This implements - LWG2762 unique_ptr operator*() should be noexcept. Differential Revision: https://reviews.llvm.org/D128214
mordante
force-pushed
the
phabricator/D128214-LWG2762
branch
from
6c3f8e6 to
712a416
Compare
mordante
force-pushed
the
phabricator/D128214-LWG2762
branch
from
3f401f2 to
2eadc6d
Compare
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Mark de Wever (mordante) ChangesThis implements
Differential Revision: https://reviews.llvm.org/D128214 Full diff: https://github.com/llvm/llvm-project/pull/98047.diff 12 Files Affected:
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..f75259473efb1 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,17 @@ _LIBCPP_PUSH_MACROS
_LIBCPP_BEGIN_NAMESPACE_STD
+#ifndef _LIBCPP_CXX03_LANG
+
+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 +265,8 @@ 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
+ _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/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/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/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..17902a4ce27a4 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,50 @@
#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>{}));
+#if TEST_STD_VER >= 11
+ static_assert(noexcept(*std::declval<std::unique_ptr<void>>()), "");
+#endif
+ {
+ 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;
}
|
|
The patch was approved on Phabricator, but had some final review comments that have been implemented in this patch. So the patch does not need another review round. |
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
This implements - LWG2762 unique_ptr operator*() should be noexcept. Differential Revision: https://reviews.llvm.org/D128214
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements
Differential Revision: https://reviews.llvm.org/D128214