Alternative to D1332

[mclow.lists]

Summary

There's a bug in the standard, where the default deleter will always call delete x; even if x is an array type. This shows up for shared_ptr<T[]>.

Do the right thing, and delete the array when you have one.

[mclow.lists]

@EricWF pointed out that this didn't actually suppress the warning that we were trying to suppress.
All I can say is that it did several weeks ago when I wrote it. Apparently clang has gotten pickier.

Use tag dispatch to choose, rather than an if. Same functionality, though.

[EricWF]

My personal preference would be to diagnose std::default_delete<T[N]> and std::unique_ptr<T[N]>, but I'll defer to your judgment here.

An alternative implementation would be to provide a "default_delete<T[N]>" specialization. (Note that adding this specialization does not change the mangling, so it is ABI safe).

include/memory

2525

__isArray doesn't match the current naming style for template parameters.

Also it would make sense to put the is_array<_Tp>::value as a default parameter here.

2527

_LIBCPP_INLINE_VISILIBITY

[STL_MSFT]

My only concern is that the tests in test/std shouldn't exercise this bug in the standard, as MSVC's implementation doesn't special-case it. If you'd like to special-case it in your product code that seems fine; this is almost to the point where the standard is so obviously broken that it should just be disregarded.

[EricWF]

If we choose to special case this I think we *need* to do it with a new specialization of default_delete<T[N]>, not using the primary template.

The primary templates defines default_delete<T[N]>::pointer as T*[N], which T* will not convert to. That means this patch still rejects things like default_delete<int[5]>{}(new int[5]) and unique_ptr<int[5]>(new int[5]) which should obviously work if we support this.

The new specialization should instead act in the same way as default_delete<T[]> and define pointer to be T*. Only after doing this do we end up with something useful.

However, We are *NOT* fixing previously buggy code. This is a pure extension. Nobody is running into bugs caused by default_delete<T[N]> calling the wrong delete method on *non-null* pointers, because only null literals convert to the pointer type`T*[N]. Non-null pointers returned from new` or malloc require a reinterpret_cast<Delete::pointer>(p) before they can be passed to the deleter. If they don't do this they will get a compile error.

IMHO instead of trying to make this work, we should diagnose it as early as possible. Any code that uses default_delete<T[N]> is going to run into a compile error pretty quickly so, we might as well make it a pretty one.

Anyway thats my 2 cents. I'll be quite now.

include/memory

 
 // default_delete
 
+template <class _Tp, bool __isArray> struct __do_default_delete;
+template <class _Tp> struct __do_default_delete<_Tp, true>
+{ void operator() (_Tp* __ptr) const _NOEXCEPT { delete [] __ptr; }};
+
+template <class _Tp> struct __do_default_delete<_Tp, false>
+{ void operator() (_Tp* __ptr) const _NOEXCEPT { delete __ptr; }};
+
 template <class _Tp>
 struct _LIBCPP_TYPE_VIS_ONLY default_delete
 {

     template <class _Up>
         _LIBCPP_INLINE_VISIBILITY default_delete(const default_delete<_Up>&,
              typename enable_if<is_convertible<_Up*, _Tp*>::value>::type* = 0) _NOEXCEPT {}
-    _LIBCPP_INLINE_VISIBILITY void operator() (_Tp* __ptr) const _NOEXCEPT
+
+    _LIBCPP_INLINE_VISIBILITY void
+    operator() (_Tp* __ptr) const _NOEXCEPT
         {
             static_assert(sizeof(_Tp) > 0, "default_delete can not delete incomplete type");
             static_assert(!is_void<_Tp>::value, "default_delete can not delete incomplete type");
-            delete __ptr;
+            __do_default_delete<_Tp, is_array<_Tp>::value>()(__ptr);
         }
 };