[libcxx-commits] [libcxx] 1f8a6dc - [libc++] Fix LWG 2874: Constructor shared_ptr::shared_ptr(Y*) should be constrained.

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Fri Apr 16 06:54:37 PDT 2021


Author: Louis Dionne
Date: 2021-04-16T09:54:20-04:00
New Revision: 1f8a6dcf128064e2ac75e4ffd227750779d83ade

URL: https://github.com/llvm/llvm-project/commit/1f8a6dcf128064e2ac75e4ffd227750779d83ade
DIFF: https://github.com/llvm/llvm-project/commit/1f8a6dcf128064e2ac75e4ffd227750779d83ade.diff

LOG: [libc++] Fix LWG 2874: Constructor shared_ptr::shared_ptr(Y*) should be constrained.

This patch fixes LWG2874. It is based on the original patch by Zoe Carver
originally uploaded at D81417.

Differential Revision: https://reviews.llvm.org/D81417

Added: 
    

Modified: 
    libcxx/docs/Cxx1zStatusIssuesStatus.csv
    libcxx/include/__memory/shared_ptr.h
    libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_Y.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/docs/Cxx1zStatusIssuesStatus.csv b/libcxx/docs/Cxx1zStatusIssuesStatus.csv
index 383c3af652d37..090e12ffd9aff 100644
--- a/libcxx/docs/Cxx1zStatusIssuesStatus.csv
+++ b/libcxx/docs/Cxx1zStatusIssuesStatus.csv
@@ -299,7 +299,7 @@
 "`2868 <https://wg21.link/LWG2868>`__","Missing specification of bad_any_cast::what()","Kona","|Complete|",""
 "`2872 <https://wg21.link/LWG2872>`__","Add definition for direct-non-list-initialization","Kona","|Complete|",""
 "`2873 <https://wg21.link/LWG2873>`__","Add noexcept to several shared_ptr related functions","Kona","|Complete|",""
-"`2874 <https://wg21.link/LWG2874>`__","Constructor ``shared_ptr::shared_ptr(Y*)``\  should be constrained","Kona","",""
+"`2874 <https://wg21.link/LWG2874>`__","Constructor ``shared_ptr::shared_ptr(Y*)``\  should be constrained","Kona","|Complete|","13.0"
 "`2875 <https://wg21.link/LWG2875>`__","shared_ptr::shared_ptr(Y\*, D, [|hellip|\ ]) constructors should be constrained","Kona","|Complete|",""
 "`2876 <https://wg21.link/LWG2876>`__","``shared_ptr::shared_ptr(const weak_ptr<Y>&)``\  constructor should be constrained","Kona","",""
 "`2878 <https://wg21.link/LWG2878>`__","Missing DefaultConstructible requirement for istream_iterator default constructor","Kona","|Complete|",""

diff  --git a/libcxx/include/__memory/shared_ptr.h b/libcxx/include/__memory/shared_ptr.h
index 8ded2ab3c7b07..e4c267b2ecade 100644
--- a/libcxx/include/__memory/shared_ptr.h
+++ b/libcxx/include/__memory/shared_ptr.h
@@ -379,6 +379,16 @@ struct __compatible_with
     : is_convertible<_Tp*, _Up*> {};
 #endif // _LIBCPP_STD_VER > 14
 
+template <class _Ptr, class = void>
+struct __is_deletable : false_type { };
+template <class _Ptr>
+struct __is_deletable<_Ptr, decltype(delete _VSTD::declval<_Ptr>())> : true_type { };
+
+template <class _Ptr, class = void>
+struct __is_array_deletable : false_type { };
+template <class _Ptr>
+struct __is_array_deletable<_Ptr, decltype(delete[] _VSTD::declval<_Ptr>())> : true_type { };
+
 template <class _Dp, class _Pt,
     class = decltype(_VSTD::declval<_Dp>()(_VSTD::declval<_Pt>()))>
 static true_type __well_formed_deleter_test(int);
@@ -424,9 +434,27 @@ class _LIBCPP_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS shared_ptr
     _LIBCPP_CONSTEXPR shared_ptr() _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY
     _LIBCPP_CONSTEXPR shared_ptr(nullptr_t) _NOEXCEPT;
-    template<class _Yp>
-        explicit shared_ptr(_Yp* __p,
-                            typename enable_if<__compatible_with<_Yp, element_type>::value, __nat>::type = __nat());
+
+    template<class _Yp, class = _EnableIf<
+        _And<
+            __compatible_with<_Yp, _Tp>
+            // In C++03 we get errors when trying to do SFINAE with the
+            // delete operator, so we always pretend that it's deletable.
+            // The same happens on GCC.
+#if !defined(_LIBCPP_CXX03_LANG) && !defined(_LIBCPP_COMPILER_GCC)
+            , _If<is_array<_Tp>::value, __is_array_deletable<_Yp*>, __is_deletable<_Yp*> >
+#endif
+        >::value
+    > >
+    explicit shared_ptr(_Yp* __p) : __ptr_(__p) {
+        unique_ptr<_Yp> __hold(__p);
+        typedef typename __shared_ptr_default_allocator<_Yp>::type _AllocT;
+        typedef __shared_ptr_pointer<_Yp*, __shared_ptr_default_delete<_Tp, _Yp>, _AllocT > _CntrlBlk;
+        __cntrl_ = new _CntrlBlk(__p, __shared_ptr_default_delete<_Tp, _Yp>(), _AllocT());
+        __hold.release();
+        __enable_weak_this(__p, __p);
+    }
+
     template<class _Yp, class _Dp>
         shared_ptr(_Yp* __p, _Dp __d,
                    typename enable_if<__shared_ptr_deleter_ctor_reqs<_Dp, _Yp, element_type>::value, __nat>::type = __nat());
@@ -678,20 +706,6 @@ shared_ptr<_Tp>::shared_ptr(nullptr_t) _NOEXCEPT
 {
 }
 
-template<class _Tp>
-template<class _Yp>
-shared_ptr<_Tp>::shared_ptr(_Yp* __p,
-                            typename enable_if<__compatible_with<_Yp, element_type>::value, __nat>::type)
-    : __ptr_(__p)
-{
-    unique_ptr<_Yp> __hold(__p);
-    typedef typename __shared_ptr_default_allocator<_Yp>::type _AllocT;
-    typedef __shared_ptr_pointer<_Yp*, __shared_ptr_default_delete<_Tp, _Yp>, _AllocT > _CntrlBlk;
-    __cntrl_ = new _CntrlBlk(__p, __shared_ptr_default_delete<_Tp, _Yp>(), _AllocT());
-    __hold.release();
-    __enable_weak_this(__p, __p);
-}
-
 template<class _Tp>
 template<class _Yp, class _Dp>
 shared_ptr<_Tp>::shared_ptr(_Yp* __p, _Dp __d,

diff  --git a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_Y.pass.cpp b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_Y.pass.cpp
index 62eb5eba1809d..9af199d376f67 100644
--- a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_Y.pass.cpp
+++ b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_Y.pass.cpp
@@ -52,6 +52,28 @@ struct C
 
 int C::count = 0;
 
+class private_delete_op
+{
+    static void operator delete (void *p) {
+        return delete static_cast<char*>(p);
+    }
+public:
+    static void operator delete[] (void *p) {
+        return delete[] static_cast<char*>(p);
+    }
+};
+
+class private_delete_arr_op
+{
+    static void operator delete[] (void *p) {
+        return delete[] static_cast<char*>(p);
+    }
+public:
+    static void operator delete (void *p) {
+        return delete static_cast<char*>(p);
+    }
+};
+
 int main(int, char**)
 {
     static_assert(( std::is_convertible<std::shared_ptr<A>, std::shared_ptr<B> >::value), "");
@@ -96,5 +118,19 @@ int main(int, char**)
     assert(B::count == 0);
     assert(A::count == 0);
 
-  return 0;
+    // This should work in C++03 but we get errors when trying to do SFINAE with the delete operator.
+    // GCC also complains about this.
+#if TEST_STD_VER >= 11 && !defined(TEST_COMPILER_GCC)
+    {
+        // LWG2874: Make sure that when T (for std::shared_ptr<T>) is an array type,
+        //          this constructor only participates in overload resolution when
+        //          `delete[] p` is well formed. And when T is not an array type,
+        //          this constructor only participates in overload resolution when
+        //          `delete p` is well formed.
+        static_assert(!std::is_constructible<std::shared_ptr<private_delete_op>,
+                                                             private_delete_op*>::value, "");
+        static_assert(!std::is_constructible<std::shared_ptr<private_delete_arr_op[4]>,
+                                                             private_delete_arr_op*>::value, "");
+    }
+#endif
 }


        


More information about the libcxx-commits mailing list