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

[Louis Dionne via Phabricator via libcxx-commits]

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

Some minor nits, that should fix the tests. LGTM once CI is passing.

Thanks for looking into this!



================
Comment at: libcxx/include/__memory/shared_ptr.h:1326
 public:
+#if _LIBCPP_STD_VER >= 20
+    typedef remove_extent_t<_Tp> element_type;
----------------



================
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:
> Quuxplusone wrote:
> > ldionne wrote:
> > > 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.
> > It would be much simpler to just treat LWG3001 as a DR against C++17. The issue description of LWG3001 makes it very clear that the //intent// was always to `remove_extent_t` in C++17; that diff just got lost in the editorial shuffle. I'm not saying "do it," but I'm at least saying "I hope @ldionne tells you to do it." ;)
> > 
> > (Personally I would be interested in a followup PR that DR'ed the `remove_extent_t` change back to C++11 just for simplicity's sake, but I suppose there'd be no real justification for that.)
> Thanks! This is also exactly what @ldionne suggested.
> 
> I'm not sure if further backporting can be done without a proposal?
Indeed, let's just DR to C++17. That's what we always do. There would be no precedent for DRing to C++11.


================
Comment at: libcxx/include/memory:529-530
 public:
-    typedef T element_type;
+    typedef T element_type; // until C++20
+    typedef remove_extent_t<T> element_type; // since C++20
 
----------------



================
Comment at: libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.weak/types.pass.cpp:14-15
 // public:
-//     typedef T element_type;
+//     typedef T element_type; // until C++20
+//     typedef remove_extent_t<T> element_type; // since C++20
 //     ...
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112092