Skip to content

Commit

Permalink
[libc++] Makes `unique_ptr operator*() noexcept.
Browse files Browse the repository at this point in the history
This implements
 - LWG2762  unique_ptr operator*() should be noexcept.

Differential Revision: https://reviews.llvm.org/D128214
  • Loading branch information
mordante committed Jul 8, 2024
1 parent f5b9e11 commit 05824f1
Show file tree
Hide file tree
Showing 11 changed files with 75 additions and 74 deletions.
2 changes: 1 addition & 1 deletion libcxx/docs/Status/Cxx23Issues.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
22 changes: 21 additions & 1 deletion libcxx/include/__memory/unique_ptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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");
Expand Down Expand Up @@ -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(); }
Expand Down
3 changes: 2 additions & 1 deletion libcxx/include/memory
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 8 additions & 17 deletions libcxx/include/optional
Original file line number Diff line number Diff line change
Expand Up @@ -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 &;
Expand Down Expand Up @@ -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());
}
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 05824f1

Please sign in to comment.