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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision.
Quuxplusone added a comment.

LGTM at this point modulo all the `>= 17` that need to be `> 14`, and that we should see buildkite passing successfully before landing this.



================
Comment at: libcxx/include/__memory/shared_ptr.h:378
 struct __compatible_with
-#if _LIBCPP_STD_VER > 14
+#if _LIBCPP_STD_VER >= 17
     : is_convertible<remove_extent_t<_Tp>*, remove_extent_t<_Up>*> {};
----------------
Please keep this as `> 14`. Even though `_LIBCPP_STD_VER` will never actually be set to "15", the intuition is that C++2b behaves more like 23 than like 20, C++1z (e.g. "C++15") behaves more like 17 than like 14, and so on. So we pretty consistently use `_LIBCPP_STD_VER > x` and `TEST_STD_VER > x` instead of `>=`.
Ditto line 422; ditto throughout (and also the tests, such as where Louis commented about it).


================
Comment at: libcxx/include/__memory/shared_ptr.h:482
     template<class _Yp> explicit shared_ptr(const weak_ptr<_Yp>& __r,
-                   typename enable_if<is_convertible<_Yp*, element_type*>::value, __nat>::type= __nat());
+                   typename enable_if<__compatible_with<_Yp, _Tp>::value, __nat>::type= __nat());
 #if _LIBCPP_STD_VER <= 14 || defined(_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR)
----------------
Pre-existing nit would be nice to fix: `type=` should be `type =`
(Also pre-existing: `typename enable_if<x>::type` could be updated to `__enable_if_t<x>`. But I recommend //against// fixing that in this PR.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112092