Fixes LWG issue 2875.
Details
Details
- Reviewers
ldionne - Group Reviewers
Restricted Project - Commits
- rGdbc89028d737: [libcxx] Fix LWG 2875: shared_ptr::shared_ptr(Y*, D, […]) constructors should…
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
Do we have tests that shared_ptr isn't constructible from incompatible array types? If not, could we add some?
Otherwise, mostly LGTM.
| libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/pointer_deleter.pass.cpp | ||
|---|---|---|
| 89 | Nit: Please don't wrap here. | |
Comment Actions
Do we have tests that shared_ptr isn't constructible from incompatible array types? If not, could we add some?
The __compatible_with trait removes the array extent. And (AFAIK) there are no array types that are convertible where their elements aren't. I'll make another patch to add tests that ensure we *can* construct a shared_ptr with array types that aren't convertible but where their elements are.
Comment Actions
LGTM. I assume the array part of the requirements in https://wg21.link/LWG2875 are handled in your arrays-in-shared-ptr patch (which I need to get back to reviewing)?
Feel free to ship it after confirmation that my understanding above is right and after CI finishes. Thanks!
Comment Actions
LGTM. I assume the array part of the requirements in https://wg21.link/LWG2875 are handled in your arrays-in-shared-ptr patch (which I need to get back to reviewing)?
I don't think the wording changes anything about how arrays are handled. As I understand it the deleter requirements are the same for array types and non-array types. Also, the arrays-in-shared-ptr patch landed: D62259. A shared_ptr with array types still uses the same overload, that's what the __compatible_with trait is in charge of.
Comment Actions
Sorry, I got confused with the patch for enabling arrays in make_shared and allocate_shared. For some reason I thought those two were more related than they actually are. Still LGTM.
| libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp | ||
|---|---|---|
| 42 | I think I'm going to remove this test and replace it with a test that just checks we hit a static_assert when instantiating default_delete with a function type; no need to get shared_ptr involved. | |
| libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp | ||
|---|---|---|
| 42 | Actually, never mind. I just removed shared_ptr from the part that was failing (because it added extra errors because a shared_ptr couldn't be constructed because it SFINAEed out). I think the other parts of this test are actually good to have. Anyway, hopefully that fixes the CI. | |