[libcxx-commits] [PATCH] D131315: [WIP] [libc++] Implement P2273R3 (`constexpr` `unique_ptr`)

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for working on this! Since the patch is marked as WIP I didn't validate everything, but in general it looks fine. However the formatting changes make it a bit hard to see all relevant changes.



================
Comment at: libcxx/include/__memory/unique_ptr.h:31
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
----------------
Please also update the synopsis in memory to mention the addition of `constexpr`.


================
Comment at: libcxx/include/__memory/unique_ptr.h:36
-    static_assert(!is_function<_Tp>::value,
-                  "default_delete cannot be instantiated for function types");
 #ifndef _LIBCPP_CXX03_LANG
----------------
In general we only format the changes in the diff instead of the entire file. That makes reviewing a bit easier.
(Unlike MSVC STL we (IMO unfortunately) haven't applied clang-format to our entire code base.)


================
Comment at: libcxx/include/__memory/unique_ptr.h:460
 template <class _T1, class _D1, class _T2, class _D2>
-inline _LIBCPP_INLINE_VISIBILITY
-bool
-operator!=(const unique_ptr<_T1, _D1>& __x, const unique_ptr<_T2, _D2>& __y) {return !(__x == __y);}
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_CXX23 bool
+operator!=(const unique_ptr<_T1, _D1>& __x, const unique_ptr<_T2, _D2>& __y) {
----------------
This isn't used in C++23.


================
Comment at: libcxx/include/__memory/unique_ptr.h:497
+typename unique_ptr<_T2, _D2>::pointer >
+    _LIBCPP_HIDE_FROM_ABI
+        compare_three_way_result_t<typename unique_ptr<_T1, _D1>::pointer, typename unique_ptr<_T2, _D2>::pointer>
----------------
You missed this one after rebasing


================
Comment at: libcxx/include/__memory/unique_ptr.h:510
 
 #if _LIBCPP_STD_VER <= 17
 template <class _T1, class _D1>
----------------
The three operators here are also not used in C++23


================
Comment at: libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/default.pass.cpp:105
     test_sfinae<int>();
-    test_basic<int>();
+    assert(test_basic<int>());
+#if TEST_STD_VER >= 23
----------------
We normally ignore the result of these tests except when we use it in a `static_assert`. Basically we use `static_assert` to force compile-time evaluation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131315