[libcxx-commits] [PATCH] D128214: [libc++] Makes `unique_ptr operator*() noexcept.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM once the comments have been addressed.



================
Comment at: libcxx/include/__memory/unique_ptr.h:272-286
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 __add_lvalue_reference_t<_Tp> operator*() const
+#if _LIBCPP_STD_VER >= 20
+      noexcept([] {
+        if constexpr (is_void_v<remove_pointer_t<pointer>>)
+          return true;
+        else
+          // This avoids evaluating noexcept(*declval<pointer>() when pointer == void*.
----------------
I think I understand the issue now. So what's going on is that the contents of `noexcept` are not SFINAE-d on, and so this becomes a hard error. A test case we should add (in all standard modes) is this (https://godbolt.org/z/Teforbb6j):

```
#include <type_traits>
#include <memory>


template <class T, class = void>
struct has_dereference : std::false_type { };

template <class T>
struct has_dereference<T, decltype((void)*std::declval<T>())> : std::true_type { };

// test the test
static_assert( has_dereference<int*>::value, "");
static_assert(!has_dereference<int>::value, "");
static_assert(!has_dereference<void*>::value, "");

// make sure std::unique_ptr<void>::operator* is SFINAE-friendly
static_assert(has_dereference<std::unique_ptr<void>>::value, "");
static_assert(noexcept(*std::declval<std::unique_ptr<void>>()), "");
```

And then the implementation should handle `void` in all standard modes, because the problem is detectable with SFINAE in all modes:

```
#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

LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX23 __add_lvalue_reference_t<_Tp> operator*() const
  _NOEXCEPT(__is_noexcept_deref_or_void<pointer>::value)
{...}
```


================
Comment at: libcxx/include/memory:452-454
     // observers
-    typename constexpr add_lvalue_reference<T>::type operator*() const;               // constexpr since C++23
+    typename constexpr
+    add_lvalue_reference<T>::type operator*() const noexcept(see below);              // constexpr since C++23
----------------
We can probably drop the `typename` from the synopsis?


================
Comment at: libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/dereference.single.pass.cpp:21
+#if TEST_STD_VER >= 11
+struct ThrowDereference {
+  TEST_CONSTEXPR_CXX23 ThrowDereference& operator*() noexcept(false);
----------------
As you mentioned, you could also add a few more test cases with user-defined pointer types. For example a `noexcept(true)` user-defined type.

Also with builtin types, e.g. `noexcept(*declval<unique_ptr<int>>())`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128214/new/

https://reviews.llvm.org/D128214