[libcxx] Correct SFINAE version of is_convertible to match spec and avoid triggering unnecessary compiler diagnostics

[ajwong]

Summary

The SFINAE forumulation used to test convertibility did not
correctly hide the conversion failure in ane expression that would
avoid a compiler diagnostic. Switched the idiom to add one more
layer of indirection and use enable_if which fixes the problem.

Also there were a few errors with const-volatile matching causing the
partial specializaitons that handled N2255 to not trigger correctly
for array types.

The following tests now compile in gcc-4.7.

futures.async/async.pass.cpp
map.cons/move_alloc.pass.cpp
map.cons/move_assign.pass.cpp
map.modifiers/insert_iter_rv.pass.cpp
map.modifiers/insert_rv.pass.cpp
meta.rel/is_convertible.pass.cpp
multimap.cons/move_alloc.pass.cpp
multimap.cons/move_assign.pass.cpp
multimap.modifiers/insert_iter_rv.pass.cpp
multimap.modifiers/insert_rv.pass.cpp
pairs.pair/rv_pair_U_V.pass.cpp
pairs.spec/make_pair.pass.cpp
thread.once.callonce/call_once.pass.cpp
unique.ptr.runtime.ctor/default02.pass.cpp
unique.ptr.runtime.ctor/pointer02.pass.cpp
unique.ptr.single.ctor/default02.pass.cpp
unique.ptr.single.ctor/pointer02.pass.cpp
unorder.map.modifiers/emplace_hint.pass.cpp
unorder.map.modifiers/emplace.pass.cpp
unorder.map.modifiers/insert_hint_rvalue.pass.cpp
unorder.map.modifiers/insert_rvalue.pass.cpp
unord.multimap.modifiers/emplace_hint.pass.cpp
unord.multimap.modifiers/emplace.pass.cpp
unord.multimap.modifiers/insert_hint_rvalue.pass.cpp
unord.multimap.modifiers/insert_rvalue.pass.cpp

[ajwong]

ping

[K-ballo]

include/type_traits

840

Couldn't __create be replaced by declval, defined earlier in this file?

842

Shouldn't this be named __helper instead?

845

If I understand this correctly, it requires generalized SFINAE to work. Is that the intention?

898

Using add_const/volatile/cv traits would probably be clearer here. Would that have any unwanted consequences?

[ajwong]

Addressed comments. PTAL

[ajwong]

gentle ping...

include/type_traits

840

Yes...I wasn't certain why this had __source() previously.

[meta.rel]p4 calls the function create(), but you're right...it looks the same. Changed and added a comment.

842

done.

845

I'm not sure I understand what generalized SFINAE is. I'm testing mainly by compiler.

The previous code just didn't work.

[danalbert]

Eric speaks template hell. Maybe he can help :)

[EricWF]

On my TODO list for tonight.

[EricWF]

The actual change looks great, and I think its almost ready to land. I would like to see more complete test coverage in meta.rel/is_convertible.pass.cpp . I understand its demonstrated by the other tests passing but I would like a localized test case for the changes.

include/type_traits

834–844

Can you add a test to meta.rel/is_convertible.pass.cpp to test this change?

887–894

I can get meta.rel/is_convertible.pass.cpp passing w/ GCC-TOT if I only apply this part of the change.

[EricWF]

Please implemented the changes requested above.

[EricWF]

This review has been inactive for a long time and this bug has already been fixed

include/type_traits

 
 template <class _T1, class _T2> struct _LIBCPP_TYPE_VIS_ONLY is_convertible
     : public integral_constant<bool, __is_convertible_to(_T1, _T2) &&
                                      !is_abstract<_T2>::value> {};
 
 #else  // __has_feature(is_convertible_to)
 
 namespace __is_convertible_imp
 {
-template <class _Tp> char  __test(_Tp);
-template <class _Tp> __two __test(...);
-#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
-template <class _Tp> _Tp&& __source();
-#else
-template <class _Tp> typename remove_reference<_Tp>::type& __source();
-#endif
+// Test derived from definition of is_convertible predicate in [meta.rel]p4.
+// declval() is used instead adding a new create() function because the
+// definitions are identical.
+template <class _Tp> char __helper(_Tp);
+
+template <class _Tp, class _Tf>
+typename enable_if<sizeof(__helper<_Tp>(declval<_Tf>())) == 1, char>::type
+    __test(int);
+template <class _Tp, class _Tf> __two __test(...);
 
 template <class _Tp, bool _IsArray =    is_array<_Tp>::value,
                      bool _IsFunction = is_function<_Tp>::value,
                      bool _IsVoid =     is_void<_Tp>::value>
                      struct __is_array_function_or_void                          {enum {value = 0};};
 template <class _Tp> struct __is_array_function_or_void<_Tp, true, false, false> {enum {value = 1};};
 template <class _Tp> struct __is_array_function_or_void<_Tp, false, true, false> {enum {value = 2};};
 template <class _Tp> struct __is_array_function_or_void<_Tp, false, false, true> {enum {value = 3};};
 }
 

 };
 
 template <class _T1, class _T2,
     unsigned _T1_is_array_function_or_void = __is_convertible_imp::__is_array_function_or_void<_T1>::value,
     unsigned _T2_is_array_function_or_void = __is_convertible_imp::__is_array_function_or_void<_T2>::value>
 struct __is_convertible
     : public integral_constant<bool,
 #ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
-        sizeof(__is_convertible_imp::__test<_T2>(__is_convertible_imp::__source<_T1>())) == 1
+        sizeof(__is_convertible_imp::__test<_T2, _T1>(1)) == 1
 #else
-        sizeof(__is_convertible_imp::__test<_T2>(__is_convertible_imp::__source<_T1>())) == 1
+        sizeof(__is_convertible_imp::__test<_T2, _T1>(1)) == 1
          && !(!is_function<_T1>::value && !is_reference<_T1>::value && is_reference<_T2>::value
               && (!is_const<typename remove_reference<_T2>::type>::value
                   || is_volatile<typename remove_reference<_T2>::type>::value)
                   && (is_same<typename remove_cv<_T1>::type,
                               typename remove_cv<typename remove_reference<_T2>::type>::type>::value
                       || is_base_of<typename remove_reference<_T2>::type, _T1>::value))
 #endif
     >
 {};
 
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 1, 0> : false_type {};
 
-template <class _T1> struct __is_convertible<_T1, const _T1&, 1, 0> : true_type {};
+template <class _T1> struct __is_convertible<_T1, typename add_const<_T1>::type&, 1, 0> : true_type {};
 #ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
 template <class _T1> struct __is_convertible<_T1, _T1&&, 1, 0> : true_type {};
-template <class _T1> struct __is_convertible<_T1, const _T1&&, 1, 0> : true_type {};
-template <class _T1> struct __is_convertible<_T1, volatile _T1&&, 1, 0> : true_type {};
-template <class _T1> struct __is_convertible<_T1, const volatile _T1&&, 1, 0> : true_type {};
+template <class _T1> struct __is_convertible<_T1, typename add_const<_T1>::type&&, 1, 0> : true_type {};
+template <class _T1> struct __is_convertible<_T1, typename add_volatile<_T1>::type&&, 1, 0> : true_type {};
+template <class _T1> struct __is_convertible<_T1, typename add_cv<_T1>::type&&, 1, 0> : true_type {};
 #endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
 
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2*, 1, 0>
     : public integral_constant<bool, __is_convertible<typename remove_all_extents<_T1>::type*, _T2*>::value> {};
 
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2* const, 1, 0>
     : public integral_constant<bool, __is_convertible<typename remove_all_extents<_T1>::type*, _T2*const>::value> {};
 
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2* volatile, 1, 0>
     : public integral_constant<bool, __is_convertible<typename remove_all_extents<_T1>::type*, _T2*volatile>::value> {};
 
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2* const volatile, 1, 0>
     : public integral_constant<bool, __is_convertible<typename remove_all_extents<_T1>::type*, _T2*const volatile>::value> {};
 
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 2, 0>                : public false_type {};
 #ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
 template <class _T1>            struct __is_convertible<_T1, _T1&&, 2, 0>               : public true_type {};
 #endif
 template <class _T1>            struct __is_convertible<_T1, _T1&, 2, 0>               : public true_type {};
 template <class _T1>            struct __is_convertible<_T1, _T1*, 2, 0>               : public true_type {};
 template <class _T1>            struct __is_convertible<_T1, _T1*const, 2, 0>          : public true_type {};
 template <class _T1>            struct __is_convertible<_T1, _T1*volatile, 2, 0>       : public true_type {};
 template <class _T1>            struct __is_convertible<_T1, _T1*const volatile, 2, 0> : public true_type {};
 
+// Per N2255 on is_convertible, void -> !void is not convertible.
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 3, 0> : public false_type {};
 
+// Per N2255 on is_convertible, * -> array is not converitble.
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 0, 1> : public false_type {};
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 1, 1> : public false_type {};
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 2, 1> : public false_type {};
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 3, 1> : public false_type {};
 
+// Per N2255 on is_convertible, * -> function is not converitble.
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 0, 2> : public false_type {};
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 1, 2> : public false_type {};
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 2, 2> : public false_type {};
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 3, 2> : public false_type {};
 
+// Per N2255 on is_convertible, only void -> void is convertible.
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 0, 3> : public false_type {};
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 1, 3> : public false_type {};
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 2, 3> : public false_type {};
 template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 3, 3> : public true_type {};
 
 template <class _T1, class _T2> struct _LIBCPP_TYPE_VIS_ONLY is_convertible
     : public __is_convertible<_T1, _T2>
 {