[libcxx-commits] [PATCH] D62274: shared_ptr deleter requirements (2802)
Louis Dionne via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Feb 17 14:49:23 PST 2021
ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.
LGTM except for the move issue (which is a really good catch BTW, I hadn't thought about that at all).
================
Comment at: libcxx/include/memory:3015
{
__d(__p);
throw;
----------------
zoecarver wrote:
> I was just thinking, is this going to be problematic if `__d` has already been moved?
Good observation. I think it's an issue, but it's really easy to fix. First, do we agree that line `3010` can't throw? Second, do we agree that we can assume move constructors don't throw?
So we only have to see whether something can throw on line `3009` after the deleter has been moved out of.
- Clearly, copying or moving `__p` around isn't an issue, since it's a pointer.
- Moving `__d` isn't an issue because we said move constructors don't throw.
- Copying `__a` is potentially problematic, since it could throw. But if we use `std::move(__a)` instead, it's not an issue anymore.
None of the stuff inside the implementation of `__shared_ptr_pointer::__shared_ptr_pointer` is an issue either, since it just moves stuff into the compressed pairs, so the same logic applies.
This reasoning applies to all 4 constructors being touched by this patch. So I think we only need to use `std::move(__a)` and we're safe even from a very pedantic point of view.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62274/new/
https://reviews.llvm.org/D62274
More information about the libcxx-commits
mailing list