[libcxx-commits] [PATCH] D143346: [libc++] fix `shared_ptr`'s incorrect constraints

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a subscriber: libc++ vendors.
ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

Thanks for fixing this! Only nits.

Let's land this and cherry-pick it onto LLVM 16. Please add a release note on the LLVM 16 release branch to explain that this constrains `std::shared_ptr` more than it used to, which could lead to a bit of source break. I don't think this should be too big of a deal, especially since the code this would break is arguably brittle.

Pinging vendors for awareness.



================
Comment at: libcxx/include/__memory/shared_ptr.h:373
+// when either Y* is convertible to T* or Y is U[N] and T is cv U[].
+#if _LIBCPP_STD_VER > 14
+template <class _Yp, class _Tp2, class = void>
----------------



================
Comment at: libcxx/include/__memory/shared_ptr.h:374-380
+template <class _Yp, class _Tp2, class = void>
+struct __bounded_convertible_to_unbounded : false_type {};
+
+template <class _Up, std::size_t _Np, class _Tp2>
+struct __bounded_convertible_to_unbounded<_Up[_Np], _Tp2,
+            __enable_if_t<__is_same(__remove_cv_t<_Tp2>, _Up[])>>
+        : true_type {};
----------------
Along with (optionally) a test case for why this can't be implemented by inheriting from `_BoolConstant`.


================
Comment at: libcxx/include/__memory/shared_ptr.h:382
+
+template <class _Yp, class _Tp2>
 struct __compatible_with
----------------
Same here, let's use `_Tp` instead of `_Tp2`.


================
Comment at: libcxx/include/__memory/shared_ptr.h:392
+    : is_convertible<_Yp*, _Tp2*> {};
+#endif // _LIBCPP_STD_VER > 14
+
----------------



================
Comment at: libcxx/include/__memory/shared_ptr.h:792
+        __compatible_with<_Yp, _Tp>,
+        is_convertible<typename unique_ptr<_Yp, _Dp>::pointer, element_type*>
+    >::value> >
----------------
Can you double-check whether this should be `element_type` or `_Tp`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143346