[libcxx-commits] [PATCH] D62274: shared_ptr deleter requirements (2802)
Zoe Carver via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Feb 17 21:10:09 PST 2021
zoecarver added inline comments.
Comment at: libcxx/include/memory:3015
> 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.
Hmm. I think move constructors //shouldn't// throw, but are allowed to. It's not undefined behavior to have a throwing move constructor (unless I'm mistaken, which is entirely possible). But I think you're right, the only thing we're really worried about is the allocator throwing. So what if we just put the call to `.allocate` inside of the try/catch? I'll update this patch to do that.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits