[PATCH] D30837: [libcxx] Support for shared_ptr<T()>

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.


================
Comment at: include/memory:3933
+    typedef __shared_ptr_pointer<_Yp*, default_delete<_Yp>, _AllocT > _CntrlBlk;
+    __cntrl_ = new _CntrlBlk(__p, default_delete<_Yp>(), _AllocT());
     __hold.release();
----------------
Quuxplusone wrote:
> EricWF wrote:
> 
> > This patch seems to support constructing a shared_ptr<FuncType> without providing a non-default deleter. I don't think this should work because the default deleter will attempt to free a function pointer, which is never valid. (Although I think this case will still cause a compile error).
> 
> Good point. But then are you suggesting that this constructor should be SFINAEd in that case, or just static_assert, or leave the existing behavior (which is indeed a hard compile error *inside* a static-assert)?
> 
> ```
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:2514:27: error: invalid application of
>       'sizeof' to a function type
>             static_assert(sizeof(_Tp) > 0, "default_delete can not delete incomplete type");
>                           ^~~~~~~~~~~
> x.cc:6:2: note: in instantiation of member function 'std::__1::default_delete<int ()>::operator()' requested here
> ```
> 
> (Technically-technically, arguably the user is allowed to fully specialize `std::default_delete<T()>` for some user-defined type `T`, right? Not that libc++ ought to be catering to such people.)
SFINAE is definitely the wrong approach, since we don't want to affect overload resolution.

On second thought it's probably better to fix the allocator type here anyway, since the instantiation of `default_delete<T>` will lead to a more readable compile error.

Also you're right about user defined specializations of default_delete, you're also right that libc++ ought not care about such craziness. 


https://reviews.llvm.org/D30837