Fix for libc++ bug #20096 http://llvm.org/bugs/show_bug.cgi?id=20096

[mclow.lists]

Summary

The existing implementation of is_destructible assumes that if a class is abstract, then it is not destructible. Apparently, the fact that you can't construct one of these objects doesn't mean that you can't destruct them.

[K-ballo]

The existing implementation is the correct one for C++11, where is_destructible<T> is defined as:

For a complete type T and given template <class U> struct test { U u; };, test<T>::˜test() is not deleted.

[mclow.lists]

Yes, but this was changed for the resolution of http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-defects.html#2049, which was a defect in C++11.

Apparently, when I worked on this issue, I missed the part about abstract classes being destructible.

[EricWF]

I reviewed the changes and are inline with the new rules. I verified the bug and testcases. This looks good to me.

[mclow.lists]

I'm getting a failure in C++03 mode; that's why I haven't committed this.

[EricWF]

test/utilities/meta/meta.unary/meta.unary.prop/is_destructible.pass.cpp

55–58

Since the destructor is private, this tests access control SFINAE and not the abstract destructor. After making the abstract destructor public, this test fails for both C++11 and C++03.

[EricWF]

Hi Marshall,

Would you be able to explain why AbstractDestructor should not be destructible?

My reading of the standard seems to suggest that because AbstractDestructor should be destructible because " decltype(_VSTD::declval<_Tp1&>().~_Tp1())" only has to be well formed in an un-evaluated context. It seems to me that invoking the destructor is well formed in this case.

Please let me know if I've made any mistakes.

[mclow.lists]

Eric - I believe that invoking the destructor in this case is not well formed, because the destructor has been explicitly deleted.

See section 8.4.2/2:
A program that refers to a deleted function implicitly or explicitly, other than to declare it, is ill-formed. [ Note: This includes calling the function implicitly or explicitly and forming a pointer or pointer-to-member to the function. It applies even for references in expressions that are not potentially-evaluated. If a function
is overloaded, it is referenced only if the function is selected by overload resolution. — end note ]

[EricWF]

Indeed, and if you test this change with a deleted destructor it is not destructible.
However I don't see why a pure virtual destructor is explicitly deleted. Can you clarify on that?

P.S. A deleted destructor test-case should probably be added.

[mclow.lists]

I don't know what the correct answer is for a publicly accessible destructor that has been marked = 0;

[mclow.lists]

A fix has been committed

include/type_traits

 
 // is_destructible
 
+//  if it's a reference, return true
+//  if it's a function, return false
+//  if it's   void,     return false
+//  if it's an array of unknown bound, return false
+//  Otherwise, return "std::declval<_Up&>().~_Up()" is well-formed
+//    where _Up is remove_all_extents<_Tp>::type
+
+template <typename _Tp>
+struct __is_destructor_wellformed {
+  template <typename _Tp1, typename _Tp2 = decltype(_VSTD::declval<_Tp1&>().~_Tp1())>
+  static char  __test (int);
+
+  template <typename _Tp1>
+  static __two __test (...);
+  
+  static const bool value = sizeof(__test<_Tp>(12)) == sizeof(char);
+  };
+
+template <class _Tp, bool>
+struct __destructible_imp;
+
 template <class _Tp>
-struct __destructible_test
-{
-    _Tp __t;
-};
+struct __destructible_imp<_Tp, false> 
+   : public _VSTD::integral_constant<bool, 
+        __is_destructor_wellformed<typename _VSTD::remove_all_extents<_Tp>::type>::value> {};
 
 template <class _Tp>
-decltype((_VSTD::declval<__destructible_test<_Tp> >().~__destructible_test<_Tp>(), true_type()))
-#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
-__is_destructible_test(_Tp&&);
-#else
-__is_destructible_test(_Tp&);
-#endif
+struct __destructible_imp<_Tp, true>
+    : public _VSTD::true_type {};
 
-false_type
-__is_destructible_test(__any);
+template <class _Tp, bool>
+struct __destructible_false;
 
-template <class _Tp, bool = is_void<_Tp>::value || is_abstract<_Tp>::value
-                                                || is_function<_Tp>::value>
-struct __destructible_imp
-    : public common_type
-        <
-            decltype(__is_destructible_test(declval<_Tp>()))
-        >::type {};
+template <class _Tp>
+struct __destructible_false<_Tp, false> : public __destructible_imp<_Tp, _VSTD::is_reference<_Tp>::value> {};
 
 template <class _Tp>
-struct __destructible_imp<_Tp, true>
-    : public false_type {};
+struct __destructible_false<_Tp, true> : public _VSTD::false_type {};
 
 template <class _Tp>
 struct is_destructible
-    : public __destructible_imp<_Tp> {};
+    : public __destructible_false<_Tp, _VSTD::is_function<_Tp>::value> {};
 
 template <class _Tp>
 struct is_destructible<_Tp[]>
-    : public false_type {};
+    : public _VSTD::false_type {};
 
+template <>
+struct is_destructible<void>
+    : public _VSTD::false_type {};
+
 // move
 
 #ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES

test/utilities/meta/meta.unary/meta.unary.prop/is_constructible.pass.cpp

     A(char);
 };
 
+class Abstract
+{
+    virtual void foo() = 0;
+};
+
+class AbstractDestructor
+{
+    virtual ~AbstractDestructor() = 0;
+};
+
 template <class T>
 void test_is_constructible()
 {

     test_is_not_constructible<A, void> ();
     test_is_not_constructible<void> ();
     test_is_not_constructible<int&> ();
+    test_is_not_constructible<Abstract> ();
+    test_is_not_constructible<AbstractDestructor> ();
 }

test/utilities/meta/meta.unary/meta.unary.prop/is_destructible.pass.cpp

 
 class Abstract
 {
-    virtual ~Abstract() = 0;
+    virtual void foo() = 0;
 };
 
+class AbstractDestructor
+{
+    virtual ~AbstractDestructor() = 0;
+};
+
 struct A
 {
     ~A();

     test_is_destructible<char[3]>();
     test_is_destructible<bit_zero>();
     test_is_destructible<int[3]>();
+    test_is_destructible<Abstract>();
 
     test_is_not_destructible<int[]>();
     test_is_not_destructible<void>();
-    test_is_not_destructible<Abstract>();
+    test_is_not_destructible<AbstractDestructor>();
 #if __has_feature(cxx_access_control_sfinae) 
     test_is_not_destructible<NotEmpty>();
 #endif