Skip to content

Commit c835fe7

Browse files
huixie90AlexisPerry
huixie90
authored and
AlexisPerry
committed
"[libc++] Try again LWG3233 Broken requirements for shared_ptr converting constructors" (llvm#96103)
Try it again. Use the approach suggested by Tim in the LWG thread : using function default argument SFINAE - Revert "[libc++] Revert LWG3233 Broken requirements for shared_ptr converting constructors (llvm#93071)" - Revert "[libc++] Revert temporary attempt to implement LWG 4110 (llvm#95263)" - test for default_delete - Revert "Revert "[libc++] Revert temporary attempt to implement LWG 4110 (llvm#95263)"" - test for NULL
1 parent d1133cf commit c835fe7

File tree

8 files changed

+154
-88
lines changed

8 files changed

+154
-88
lines changed

‎libcxx/docs/Status/Cxx20Issues.csv

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@
200200
"`3200 <https://wg21.link/LWG3200>`__","``midpoint``\ should not constrain ``T``\ is complete","Prague","|Nothing To Do|",""
201201
"`3201 <https://wg21.link/LWG3201>`__","``lerp``\ should be marked as ``noexcept``\ ","Prague","|Complete|",""
202202
"`3226 <https://wg21.link/LWG3226>`__","``zoned_time``\ constructor from ``string_view``\ should accept ``zoned_time<Duration2, TimeZonePtr2>``\ ","Prague","","","|chrono|"
203-
"`3233 <https://wg21.link/LWG3233>`__","Broken requirements for ``shared_ptr``\ converting constructors","Prague","",""
203+
"`3233 <https://wg21.link/LWG3233>`__","Broken requirements for ``shared_ptr``\ converting constructors","Prague","|Complete|","19.0"
204204
"`3237 <https://wg21.link/LWG3237>`__","LWG 3038 and 3190 have inconsistent PRs","Prague","|Complete|","16.0"
205205
"`3238 <https://wg21.link/LWG3238>`__","Insufficiently-defined behavior of ``std::function``\ deduction guides","Prague","|Nothing To Do|",""
206206
"`3242 <https://wg21.link/LWG3242>`__","``std::format``\ : missing rules for ``arg-id``\ in ``width``\ and ``precision``\ ","Prague","|Complete|","14.0","|format|"

‎libcxx/include/__memory/shared_ptr.h

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,9 @@ struct __shared_ptr_deleter_ctor_reqs {
403403
__well_formed_deleter<_Dp, _Yp*>::value;
404404
};
405405

406+
template <class _Dp>
407+
using __shared_ptr_nullptr_deleter_ctor_reqs = _And<is_move_constructible<_Dp>, __well_formed_deleter<_Dp, nullptr_t> >;
408+
406409
#if defined(_LIBCPP_ABI_ENABLE_SHARED_PTR_TRIVIAL_ABI)
407410
# define _LIBCPP_SHARED_PTR_TRIVIAL_ABI __attribute__((__trivial_abi__))
408411
#else
@@ -411,6 +414,8 @@ struct __shared_ptr_deleter_ctor_reqs {
411414

412415
template <class _Tp>
413416
class _LIBCPP_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS shared_ptr {
417+
struct __nullptr_sfinae_tag {};
418+
414419
public:
415420
#if _LIBCPP_STD_VER >= 17
416421
typedef weak_ptr<_Tp> weak_type;
@@ -503,7 +508,11 @@ class _LIBCPP_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS shared_ptr {
503508
}
504509

505510
template <class _Dp>
506-
_LIBCPP_HIDE_FROM_ABI shared_ptr(nullptr_t __p, _Dp __d) : __ptr_(nullptr) {
511+
_LIBCPP_HIDE_FROM_ABI shared_ptr(
512+
nullptr_t __p,
513+
_Dp __d,
514+
__enable_if_t<__shared_ptr_nullptr_deleter_ctor_reqs<_Dp>::value, __nullptr_sfinae_tag> = __nullptr_sfinae_tag())
515+
: __ptr_(nullptr) {
507516
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
508517
try {
509518
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
@@ -523,7 +532,12 @@ class _LIBCPP_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS shared_ptr {
523532
}
524533

525534
template <class _Dp, class _Alloc>
526-
_LIBCPP_HIDE_FROM_ABI shared_ptr(nullptr_t __p, _Dp __d, _Alloc __a) : __ptr_(nullptr) {
535+
_LIBCPP_HIDE_FROM_ABI shared_ptr(
536+
nullptr_t __p,
537+
_Dp __d,
538+
_Alloc __a,
539+
__enable_if_t<__shared_ptr_nullptr_deleter_ctor_reqs<_Dp>::value, __nullptr_sfinae_tag> = __nullptr_sfinae_tag())
540+
: __ptr_(nullptr) {
527541
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
528542
try {
529543
#endif // _LIBCPP_HAS_NO_EXCEPTIONS

‎libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/nullptr_t_deleter.pass.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "test_macros.h"
1818
#include "deleter_types.h"
1919

20+
#include "types.h"
2021
struct A
2122
{
2223
static int count;
@@ -28,6 +29,25 @@ struct A
2829

2930
int A::count = 0;
3031

32+
// LWG 3233. Broken requirements for shared_ptr converting constructors
33+
// https://cplusplus.github.io/LWG/issue3233
34+
static_assert( std::is_constructible<std::shared_ptr<int>, std::nullptr_t, test_deleter<int> >::value, "");
35+
static_assert(!std::is_constructible<std::shared_ptr<int>, std::nullptr_t, bad_deleter>::value, "");
36+
static_assert(!std::is_constructible<std::shared_ptr<int>, std::nullptr_t, no_nullptr_deleter>::value, "");
37+
static_assert(!std::is_constructible<std::shared_ptr<int>, std::nullptr_t, no_move_deleter>::value, "");
38+
39+
#if TEST_STD_VER >= 17
40+
static_assert( std::is_constructible<std::shared_ptr<int[]>, std::nullptr_t, test_deleter<int> >::value, "");
41+
static_assert(!std::is_constructible<std::shared_ptr<int[]>, std::nullptr_t, bad_deleter>::value, "");
42+
static_assert(!std::is_constructible<std::shared_ptr<int[]>, std::nullptr_t, no_nullptr_deleter>::value, "");
43+
static_assert(!std::is_constructible<std::shared_ptr<int[]>, std::nullptr_t, no_move_deleter>::value, "");
44+
45+
static_assert( std::is_constructible<std::shared_ptr<int[5]>, std::nullptr_t, test_deleter<int> >::value, "");
46+
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, std::nullptr_t, bad_deleter>::value, "");
47+
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, std::nullptr_t, no_nullptr_deleter>::value, "");
48+
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, std::nullptr_t, no_move_deleter>::value, "");
49+
#endif
50+
3151
int main(int, char**)
3252
{
3353
{

‎libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/nullptr_t_deleter_allocator.pass.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
#include "test_allocator.h"
1818
#include "min_allocator.h"
1919

20+
#include "types.h"
21+
2022
struct A
2123
{
2224
static int count;
@@ -28,6 +30,25 @@ struct A
2830

2931
int A::count = 0;
3032

33+
// LWG 3233. Broken requirements for shared_ptr converting constructors
34+
// https://cplusplus.github.io/LWG/issue3233
35+
static_assert( std::is_constructible<std::shared_ptr<int>, std::nullptr_t, test_deleter<int>, test_allocator<int> >::value, "");
36+
static_assert(!std::is_constructible<std::shared_ptr<int>, std::nullptr_t, bad_deleter, test_allocator<int> >::value, "");
37+
static_assert(!std::is_constructible<std::shared_ptr<int>, std::nullptr_t, no_nullptr_deleter, test_allocator<int> >::value, "");
38+
static_assert(!std::is_constructible<std::shared_ptr<int>, std::nullptr_t, no_move_deleter, test_allocator<int> >::value, "");
39+
40+
#if TEST_STD_VER >= 17
41+
static_assert( std::is_constructible<std::shared_ptr<int[]>, std::nullptr_t, test_deleter<int>, test_allocator<int> >::value, "");
42+
static_assert(!std::is_constructible<std::shared_ptr<int[]>, std::nullptr_t, bad_deleter, test_allocator<int> >::value, "");
43+
static_assert(!std::is_constructible<std::shared_ptr<int[]>, std::nullptr_t, no_nullptr_deleter, test_allocator<int> >::value, "");
44+
static_assert(!std::is_constructible<std::shared_ptr<int[]>, std::nullptr_t, no_move_deleter, test_allocator<int> >::value, "");
45+
46+
static_assert( std::is_constructible<std::shared_ptr<int[5]>, std::nullptr_t, test_deleter<int>, test_allocator<int> >::value, "");
47+
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, std::nullptr_t, bad_deleter, test_allocator<int> >::value, "");
48+
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, std::nullptr_t, no_nullptr_deleter, test_allocator<int> >::value, "");
49+
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, std::nullptr_t, no_move_deleter, test_allocator<int> >::value, "");
50+
#endif
51+
3152
int main(int, char**)
3253
{
3354
test_allocator_statistics alloc_stats;

‎libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/pointer.pass.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ struct A
2727

2828
int A::count = 0;
2929

30-
struct Derived : A {};
30+
struct derived : A {};
3131

3232
// https://llvm.org/PR60258
3333
// Invalid constructor SFINAE for std::shared_ptr's array ctors
3434
static_assert( std::is_constructible<std::shared_ptr<int>, int*>::value, "");
35-
static_assert( std::is_constructible<std::shared_ptr<A>, Derived*>::value, "");
35+
static_assert( std::is_constructible<std::shared_ptr<A>, derived*>::value, "");
3636
static_assert(!std::is_constructible<std::shared_ptr<A>, int*>::value, "");
3737

3838
#if TEST_STD_VER >= 17
@@ -99,7 +99,7 @@ int main(int, char**)
9999

100100
{
101101
assert(A::count == 0);
102-
std::shared_ptr<A> pA(new Derived);
102+
std::shared_ptr<A> pA(new derived);
103103
assert(pA.use_count() == 1);
104104
assert(A::count == 1);
105105
}

‎libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/pointer_deleter.pass.cpp

Lines changed: 30 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
#include "test_macros.h"
1818
#include "deleter_types.h"
1919

20+
#include "types.h"
21+
2022
struct A
2123
{
2224
static int count;
@@ -28,52 +30,22 @@ struct A
2830

2931
int A::count = 0;
3032

31-
struct bad_ty { };
32-
33-
struct bad_deleter
34-
{
35-
void operator()(bad_ty) { }
36-
};
37-
38-
struct no_move_deleter
39-
{
40-
no_move_deleter(no_move_deleter const&) = delete;
41-
no_move_deleter(no_move_deleter &&) = delete;
42-
void operator()(int*) { }
43-
};
44-
45-
static_assert(!std::is_move_constructible<no_move_deleter>::value, "");
46-
47-
struct Base { };
48-
struct Derived : Base { };
49-
50-
template<class T>
51-
class MoveDeleter
52-
{
53-
MoveDeleter();
54-
MoveDeleter(MoveDeleter const&);
55-
public:
56-
MoveDeleter(MoveDeleter&&) {}
57-
58-
explicit MoveDeleter(int) {}
59-
60-
void operator()(T* ptr) { delete ptr; }
61-
};
62-
33+
// LWG 3233. Broken requirements for shared_ptr converting constructors
34+
// https://cplusplus.github.io/LWG/issue3233
6335
// https://llvm.org/PR60258
6436
// Invalid constructor SFINAE for std::shared_ptr's array ctors
6537
static_assert( std::is_constructible<std::shared_ptr<int>, int*, test_deleter<int> >::value, "");
6638
static_assert(!std::is_constructible<std::shared_ptr<int>, int*, bad_deleter>::value, "");
67-
static_assert( std::is_constructible<std::shared_ptr<Base>, Derived*, test_deleter<Base> >::value, "");
39+
static_assert( std::is_constructible<std::shared_ptr<base>, derived*, test_deleter<base> >::value, "");
6840
static_assert(!std::is_constructible<std::shared_ptr<A>, int*, test_deleter<A> >::value, "");
6941

7042
#if TEST_STD_VER >= 17
71-
static_assert( std::is_constructible<std::shared_ptr<int[]>, int*, test_deleter<int>>::value, "");
43+
static_assert( std::is_constructible<std::shared_ptr<int[]>, int*, test_deleter<int> >::value, "");
7244
static_assert(!std::is_constructible<std::shared_ptr<int[]>, int*, bad_deleter>::value, "");
73-
static_assert(!std::is_constructible<std::shared_ptr<int[]>, int(*)[], test_deleter<int>>::value, "");
74-
static_assert( std::is_constructible<std::shared_ptr<int[5]>, int*, test_deleter<int>>::value, "");
45+
static_assert(!std::is_constructible<std::shared_ptr<int[]>, int(*)[], test_deleter<int> >::value, "");
46+
static_assert( std::is_constructible<std::shared_ptr<int[5]>, int*, test_deleter<int> >::value, "");
7547
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, int*, bad_deleter>::value, "");
76-
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, int(*)[5], test_deleter<int>>::value, "");
48+
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, int(*)[5], test_deleter<int> >::value, "");
7749
#endif
7850

7951
int f() { return 5; }
@@ -132,17 +104,36 @@ int main(int, char**)
132104

133105
// Make sure that we can construct a shared_ptr where the element type and pointer type
134106
// aren't "convertible" but are "compatible".
135-
static_assert(!std::is_constructible<std::shared_ptr<Derived[4]>, Base[4], test_deleter<Derived[4]> >::value, "");
107+
static_assert(!std::is_constructible<std::shared_ptr<derived[4]>, base[4], test_deleter<derived[4]> >::value, "");
136108
}
137109

138110
#if TEST_STD_VER >= 11
139111
{
140-
MoveDeleter<int> d(0);
112+
move_deleter<int> d(0);
141113
std::shared_ptr<int> p0(new int, std::move(d));
142114
std::shared_ptr<int> p1(nullptr, std::move(d));
143115
}
144116
#endif // TEST_STD_VER >= 11
145117

118+
#if TEST_STD_VER >= 14
119+
{
120+
// LWG 4110
121+
auto deleter = [](auto pointer) { delete pointer; };
122+
std::shared_ptr<int> p(new int, deleter);
123+
}
124+
125+
{
126+
std::shared_ptr<int> p(NULL, [](auto){});
127+
}
128+
#endif
129+
130+
#if TEST_STD_VER >= 17
131+
{
132+
// See https://github.com/llvm/llvm-project/pull/93071#issuecomment-2166047398
133+
std::shared_ptr<char[]> a(new char[10], std::default_delete<char[]>());
134+
}
135+
#endif
136+
146137
test_function_type();
147138
return 0;
148139
}

‎libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/pointer_deleter_allocator.pass.cpp

Lines changed: 14 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "test_allocator.h"
1818
#include "min_allocator.h"
1919

20+
#include "types.h"
2021
struct A
2122
{
2223
static int count;
@@ -28,52 +29,22 @@ struct A
2829

2930
int A::count = 0;
3031

31-
struct bad_ty { };
32-
33-
struct bad_deleter
34-
{
35-
void operator()(bad_ty) { }
36-
};
37-
38-
struct no_move_deleter
39-
{
40-
no_move_deleter(no_move_deleter const&) = delete;
41-
no_move_deleter(no_move_deleter &&) = delete;
42-
void operator()(int*) { }
43-
};
44-
45-
static_assert(!std::is_move_constructible<no_move_deleter>::value, "");
46-
47-
struct Base { };
48-
struct Derived : Base { };
49-
50-
template<class T>
51-
class MoveDeleter
52-
{
53-
MoveDeleter();
54-
MoveDeleter(MoveDeleter const&);
55-
public:
56-
MoveDeleter(MoveDeleter&&) {}
57-
58-
explicit MoveDeleter(int) {}
59-
60-
void operator()(T* ptr) { delete ptr; }
61-
};
62-
32+
// LWG 3233. Broken requirements for shared_ptr converting constructors
33+
// https://cplusplus.github.io/LWG/issue3233
6334
// https://llvm.org/PR60258
6435
// Invalid constructor SFINAE for std::shared_ptr's array ctors
6536
static_assert( std::is_constructible<std::shared_ptr<int>, int*, test_deleter<int>, test_allocator<int> >::value, "");
6637
static_assert(!std::is_constructible<std::shared_ptr<int>, int*, bad_deleter, test_allocator<int> >::value, "");
67-
static_assert( std::is_constructible<std::shared_ptr<Base>, Derived*, test_deleter<Base>, test_allocator<Base> >::value, "");
38+
static_assert( std::is_constructible<std::shared_ptr<base>, derived*, test_deleter<base>, test_allocator<base> >::value, "");
6839
static_assert(!std::is_constructible<std::shared_ptr<A>, int*, test_deleter<A>, test_allocator<A> >::value, "");
6940

7041
#if TEST_STD_VER >= 17
71-
static_assert( std::is_constructible<std::shared_ptr<int[]>, int*, test_deleter<int>, test_allocator<int>>::value, "");
72-
static_assert(!std::is_constructible<std::shared_ptr<int[]>, int*, bad_deleter, test_allocator<int>>::value, "");
73-
static_assert(!std::is_constructible<std::shared_ptr<int[]>, int(*)[], test_deleter<int>, test_allocator<int>>::value, "");
74-
static_assert( std::is_constructible<std::shared_ptr<int[5]>, int*, test_deleter<int>, test_allocator<int>>::value, "");
75-
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, int*, bad_deleter, test_allocator<int>>::value, "");
76-
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, int(*)[5], test_deleter<int>, test_allocator<int>>::value, "");
42+
static_assert( std::is_constructible<std::shared_ptr<int[]>, int*, test_deleter<int>, test_allocator<int> >::value, "");
43+
static_assert(!std::is_constructible<std::shared_ptr<int[]>, int*, bad_deleter, test_allocator<int> >::value, "");
44+
static_assert(!std::is_constructible<std::shared_ptr<int[]>, int(*)[], test_deleter<int>, test_allocator<int> >::value, "");
45+
static_assert( std::is_constructible<std::shared_ptr<int[5]>, int*, test_deleter<int>, test_allocator<int> >::value, "");
46+
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, int*, bad_deleter, test_allocator<int> >::value, "");
47+
static_assert(!std::is_constructible<std::shared_ptr<int[5]>, int(*)[5], test_deleter<int>, test_allocator<int> >::value, "");
7748
#endif
7849

7950

@@ -172,7 +143,7 @@ int main(int, char**)
172143
assert(test_deleter<A>::dealloc_count == 1);
173144

174145
{
175-
MoveDeleter<int> d(0);
146+
move_deleter<int> d(0);
176147
std::shared_ptr<int> p2(new int, std::move(d), std::allocator<int>());
177148
std::shared_ptr<int> p3(nullptr, std::move(d), std::allocator<int>());
178149
}
@@ -189,9 +160,9 @@ int main(int, char**)
189160

190161
// Make sure that we can construct a shared_ptr where the element type and pointer type
191162
// aren't "convertible" but are "compatible".
192-
static_assert(!std::is_constructible<std::shared_ptr<Derived[4]>,
193-
Base[4], test_deleter<Derived[4]>,
194-
test_allocator<Derived[4]> >::value, "");
163+
static_assert(!std::is_constructible<std::shared_ptr<derived[4]>,
164+
base[4], test_deleter<derived[4]>,
165+
test_allocator<derived[4]> >::value, "");
195166
}
196167

197168
return 0;

‎libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/types.h

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef TEST_STD_UTILITIES_MEMORY_UTIL_SMARTPTR_SHARED_CONST_TYPES_H
10+
#define TEST_STD_UTILITIES_MEMORY_UTIL_SMARTPTR_SHARED_CONST_TYPES_H
11+
12+
#include <type_traits>
13+
14+
struct bad_type {};
15+
16+
struct bad_deleter {
17+
void operator()(bad_type) {}
18+
};
19+
20+
struct no_move_deleter {
21+
no_move_deleter(no_move_deleter const&) = delete;
22+
no_move_deleter(no_move_deleter&&) = delete;
23+
void operator()(int*) {}
24+
};
25+
26+
static_assert(!std::is_move_constructible<no_move_deleter>::value, "");
27+
28+
struct no_nullptr_deleter {
29+
void operator()(int*) const {}
30+
void operator()(std::nullptr_t) const = delete;
31+
};
32+
33+
struct base {};
34+
struct derived : base {};
35+
36+
template <class T>
37+
class move_deleter {
38+
move_deleter();
39+
move_deleter(move_deleter const&);
40+
41+
public:
42+
move_deleter(move_deleter&&) {}
43+
44+
explicit move_deleter(int) {}
45+
46+
void operator()(T* ptr) { delete ptr; }
47+
};
48+
49+
#endif // TEST_STD_UTILITIES_MEMORY_UTIL_SMARTPTR_SHARED_CONST_TYPES_H

0 commit comments

Comments
 (0)