[libcxx-commits] [PATCH] D112092: [libc++] LWG3001: add `remove_extent_t` to `weak_ptr::element_type`.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Thanks for looking into this! Have a few comments but it LGTM in essence, once we've addressed the LWG issue backporting. I think this should simplify the patch a bit.



================
Comment at: libcxx/include/__memory/shared_ptr.h:384-410
+template<class _Tp, class _Up>
+struct __weak_ptr_compatible_with
+#if _LIBCPP_STD_VER > 17
+    : is_convertible<remove_extent_t<_Tp>*, remove_extent_t<_Up>*> {};
+#else
+    : is_convertible<_Tp*, _Up*> {};
+#endif // _LIBCPP_STD_VER > 17
----------------
var-const wrote:
> This is pretty verbose, but it seemed important to ensure no changes in behavior for each language version. Please let me know if you have any suggestions on how to improve this.
Per offline review: Since this is a LWG issue we're fixing, we normally apply it in all standard modes. So you can fix `weak_ptr` in C++17 and above in-place.


================
Comment at: libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/weak_ptr.pass.cpp:85
 
+#if TEST_STD_VER > 17
+    {
----------------
You'll want to change this to `#if TEST_STD_VER > 14` per my comment about backporting LWG issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112092