This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: functional and type_traits cleanup


On 04/05/2013 12:20 AM, Jonathan Wakely wrote:
On 4 April 2013 21:16, François Dumont wrote:
I think this is mostly very good, thanks for cleaning it up. The indentiation of the closing brace for __is_assignable_helper looks wrong. Is there a reason that __is_assignable_helper::__test uses a default template argument but __is_convertible_helper::__test uses decltype(expr, type) in the function return type? I think the decltype(__test_aux<_Tp1>(...)) expression would work as a default template argument too, which I find easier to read because it doesn't clutter up the return type.

In fact my first attempt was a very simple one:

  template<typename _From, typename _To>
    class __is_convertible_helper<_From, _To, false>
    {
      template<typename _To1>
    static true_type
    __test(_To1);

      template<typename>
    static false_type
    __test(...);

    public:
      typedef decltype(__test<_To>(std::declval<_From>())) type;
    };

But some tests failed like:
In file included from /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/bits/move.h:57:0, from /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/bits/stl_pair.h:59, from /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/utility:70, from /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/tuple:38, from /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/functional:55, from /home/fdt/dev/gcc/src/libstdc++-v3/testsuite/20_util/bind/38889.cc:23: /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/type_traits: In instantiation of 'struct std::__is_convertible_helper<const std::tuple<std::_Placeholder<1> >&, std::_Placeholder<1>, false>': /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/type_traits:1321:12: required from 'struct std::is_convertible<const std::tuple<std::_Placeholder<1> >&, std::_Placeholder<1> >' /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/type_traits:111:12: required from 'struct std::__and_<std::is_convertible<const std::tuple<std::_Placeholder<1> >&, std::_Placeholder<1> > >' /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/tuple:400:40: required from 'struct std::_Bind<void (*(std::_Placeholder<1>))(int)>' /home/fdt/dev/gcc/src/libstdc++-v3/testsuite/20_util/bind/38889.cc:28:41: required from here /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/type_traits:1316:30: error: 'std::_Placeholder<1>' is an inaccessible base of 'std::tuple<std::_Placeholder<1> >'
       typedef decltype(__test<_To>(std::declval<_From>())) type;
                              ^
/home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/type_traits:1309:2: error: initializing argument 1 of 'static std::true_type std::__is_convertible_helper<_From, _To, false>::__test(_To1) [with _To1 = std::_Placeholder<1>; _From = const std::tuple<std::_Placeholder<1> >&; _To = std::_Placeholder<1>; std::true_type = std::integral_constant<bool, true>]'
  __test(_To1);
  ^

From my point of view this is an other example of use case for which gcc is not SFINAE friendly enough, no ?

But the version with the default template parameter is fine and more consistent with the other helpers implementation so, adopted! Here is an other version of the patch for validation.

Daniel, I agree that inheritance with integral_constant is not as obvious as before but it is still there and it is just what the compiler need. I even hope that it also simplified a (very) little bit the job for the compiler.

Ok to commit ?

François

Index: include/std/functional
===================================================================
--- include/std/functional	(revision 197307)
+++ include/std/functional	(working copy)
@@ -185,38 +185,6 @@
     : _Weak_result_type_impl<typename remove_cv<_Functor>::type>
     { };
 
-  /// Determines if the type _Tp derives from unary_function.
-  template<typename _Tp>
-    struct _Derives_from_unary_function : __sfinae_types
-    {
-    private:
-      template<typename _T1, typename _Res>
-	static __one __test(const volatile unary_function<_T1, _Res>*);
-
-      // It's tempting to change "..." to const volatile void*, but
-      // that fails when _Tp is a function type.
-      static __two __test(...);
-
-    public:
-      static const bool value = sizeof(__test((_Tp*)0)) == 1;
-    };
-
-  /// Determines if the type _Tp derives from binary_function.
-  template<typename _Tp>
-    struct _Derives_from_binary_function : __sfinae_types
-    {
-    private:
-      template<typename _T1, typename _T2, typename _Res>
-	static __one __test(const volatile binary_function<_T1, _T2, _Res>*);
-
-      // It's tempting to change "..." to const volatile void*, but
-      // that fails when _Tp is a function type.
-      static __two __test(...);
-
-    public:
-      static const bool value = sizeof(__test((_Tp*)0)) == 1;
-    };
-
   /**
    * Invoke a function object, which may be either a member pointer or a
    * function object. The first parameter will tell which.
Index: include/std/type_traits
===================================================================
--- include/std/type_traits	(revision 197307)
+++ include/std/type_traits	(working copy)
@@ -127,12 +127,6 @@
     : public integral_constant<bool, !_Pp::value>
     { };
 
-  struct __sfinae_types
-  {
-    typedef char __one;
-    typedef struct { char __arr[2]; } __two;
-  };
-
   // For several sfinae-friendly trait implementations we transport both the
   // result information (as the member type) and the failure information (no
   // member type). This is very similar to std::enable_if, but we cannot use
@@ -161,8 +155,7 @@
   /// is_void
   template<typename _Tp>
     struct is_void
-    : public integral_constant<bool, (__is_void_helper<typename
-				      remove_cv<_Tp>::type>::value)>
+    : public __is_void_helper<typename remove_cv<_Tp>::type>::type
     { };
 
   template<typename>
@@ -244,8 +237,7 @@
   /// is_integral
   template<typename _Tp>
     struct is_integral
-    : public integral_constant<bool, (__is_integral_helper<typename
-				      remove_cv<_Tp>::type>::value)>
+    : public __is_integral_helper<typename remove_cv<_Tp>::type>::type
     { };
 
   template<typename>
@@ -273,8 +265,7 @@
   /// is_floating_point
   template<typename _Tp>
     struct is_floating_point
-    : public integral_constant<bool, (__is_floating_point_helper<typename
-				      remove_cv<_Tp>::type>::value)>
+    : public __is_floating_point_helper<typename remove_cv<_Tp>::type>::type
     { };
 
   /// is_array
@@ -301,8 +292,7 @@
   /// is_pointer
   template<typename _Tp>
     struct is_pointer
-    : public integral_constant<bool, (__is_pointer_helper<typename
-				      remove_cv<_Tp>::type>::value)>
+    : public __is_pointer_helper<typename remove_cv<_Tp>::type>::type
     { };
 
   /// is_lvalue_reference
@@ -337,8 +327,8 @@
   /// is_member_object_pointer
   template<typename _Tp>
     struct is_member_object_pointer
-    : public integral_constant<bool, (__is_member_object_pointer_helper<
-				      typename remove_cv<_Tp>::type>::value)>
+    : public __is_member_object_pointer_helper<
+				typename remove_cv<_Tp>::type>::type
     { };
 
   template<typename>
@@ -352,8 +342,8 @@
   /// is_member_function_pointer
   template<typename _Tp>
     struct is_member_function_pointer
-    : public integral_constant<bool, (__is_member_function_pointer_helper<
-				      typename remove_cv<_Tp>::type>::value)>
+    : public __is_member_function_pointer_helper<
+				typename remove_cv<_Tp>::type>::type
     { };
 
   /// is_enum
@@ -422,8 +412,7 @@
   // __is_nullptr_t (extension).
   template<typename _Tp>
     struct __is_nullptr_t
-    : public integral_constant<bool, (__is_nullptr_t_helper<typename
-				      remove_cv<_Tp>::type>::value)>
+    : public __is_nullptr_t_helper<typename remove_cv<_Tp>::type>::type
     { };
 
   // Composite type categories.
@@ -480,8 +469,7 @@
   /// is_member_pointer
   template<typename _Tp>
     struct is_member_pointer
-    : public integral_constant<bool, (__is_member_pointer_helper<
-				      typename remove_cv<_Tp>::type>::value)>
+    : public __is_member_pointer_helper<typename remove_cv<_Tp>::type>::type
     { };
 
   // Type properties.
@@ -567,7 +555,7 @@
   /// is_signed
   template<typename _Tp>
     struct is_signed
-    : public integral_constant<bool, __is_signed_helper<_Tp>::value>
+    : public __is_signed_helper<_Tp>::type
     { };
 
   /// is_unsigned
@@ -650,7 +638,7 @@
   /// is_destructible
   template<typename _Tp>
     struct is_destructible
-    : public integral_constant<bool, (__is_destructible_safe<_Tp>::value)>
+    : public __is_destructible_safe<_Tp>::type
     { };
 
   // is_nothrow_destructible requires that is_destructible is
@@ -698,7 +686,7 @@
   /// is_nothrow_destructible
   template<typename _Tp>
     struct is_nothrow_destructible
-    : public integral_constant<bool, (__is_nt_destructible_safe<_Tp>::value)>
+    : public __is_nt_destructible_safe<_Tp>::type
     { };
 
   struct __do_is_default_constructible_impl
@@ -746,8 +734,7 @@
   /// is_default_constructible
   template<typename _Tp>
     struct is_default_constructible
-    : public integral_constant<bool, (__is_default_constructible_safe<
-				      _Tp>::value)>
+    : public __is_default_constructible_safe<_Tp>::type
     { };
 
 
@@ -901,8 +888,7 @@
 
   template<typename _Tp, typename _Arg>
     struct __is_direct_constructible
-    : public integral_constant<bool, (__is_direct_constructible_new<
-				      _Tp, _Arg>::value)>
+    : public __is_direct_constructible_new<_Tp, _Arg>::type
     { };
 
   // Since default-construction and binary direct-initialization have
@@ -953,8 +939,7 @@
   /// is_constructible
   template<typename _Tp, typename... _Args>
     struct is_constructible
-    : public integral_constant<bool, (__is_constructible_impl<_Tp,
-				      _Args...>::value)>
+    : public __is_constructible_impl<_Tp, _Args...>::type
     { };
 
   template<typename _Tp, bool = is_void<_Tp>::value>
@@ -1081,24 +1066,24 @@
 
   template<typename _Tp, typename _Up>
     class __is_assignable_helper
-    : public __sfinae_types
     {
-      template<typename _Tp1, typename _Up1>
-        static decltype(declval<_Tp1>() = declval<_Up1>(), __one())
+      template<typename _Tp1, typename _Up1,
+	       typename = decltype(declval<_Tp1>() = declval<_Up1>())>
+	static true_type
 	__test(int);
 
       template<typename, typename>
-        static __two __test(...);
+	static false_type
+	__test(...);
 
     public:
-      static constexpr bool value = sizeof(__test<_Tp, _Up>(0)) == 1;
+      typedef decltype(__test<_Tp, _Up>(0)) type;
     };
 
   /// is_assignable
   template<typename _Tp, typename _Up>
     struct is_assignable
-    : public integral_constant<bool,
-                               __is_assignable_helper<_Tp, _Up>::value>
+      : public __is_assignable_helper<_Tp, _Up>::type
     { };
 
   template<typename _Tp, bool = is_void<_Tp>::value>
@@ -1292,31 +1277,32 @@
            bool = __or_<is_void<_From>, is_function<_To>,
                         is_array<_To>>::value>
     struct __is_convertible_helper
-    { static constexpr bool value = is_void<_To>::value; };
+    { typedef typename is_void<_To>::type type; };
 
   template<typename _From, typename _To>
     class __is_convertible_helper<_From, _To, false>
-    : public __sfinae_types
     {
-      template<typename _To1>
-        static void __test_aux(_To1);
+       template<typename _To1>
+	static void __test_aux(_To1);
 
-      template<typename _From1, typename _To1>
-        static decltype(__test_aux<_To1>(std::declval<_From1>()), __one())
+      template<typename _From1, typename _To1,
+	       typename = decltype(__test_aux<_To1>(std::declval<_From1>()))>
+	static true_type
 	__test(int);
 
       template<typename, typename>
-        static __two __test(...);
+	static false_type
+	__test(...);
 
     public:
-      static constexpr bool value = sizeof(__test<_From, _To>(0)) == 1;
+      typedef decltype(__test<_From, _To>(0)) type;
     };
 
+
   /// is_convertible
   template<typename _From, typename _To>
     struct is_convertible
-    : public integral_constant<bool,
-			       __is_convertible_helper<_From, _To>::value>
+    : public __is_convertible_helper<_From, _To>::type
     { };
 
 
@@ -2041,29 +2027,28 @@
    *  Use SFINAE to determine if the type _Tp has a publicly-accessible
    *  member type _NTYPE.
    */
-#define _GLIBCXX_HAS_NESTED_TYPE(_NTYPE)                         \
-  template<typename _Tp>                                         \
-    class __has_##_NTYPE##_helper                                \
-    : __sfinae_types                                             \
-    {                                                            \
-      template<typename _Up>                                     \
-        struct _Wrap_type                                        \
-	{ };                                                     \
-                                                                 \
-      template<typename _Up>                                     \
-        static __one __test(_Wrap_type<typename _Up::_NTYPE>*);  \
-                                                                 \
-      template<typename _Up>                                     \
-        static __two __test(...);                                \
-                                                                 \
-    public:                                                      \
-      static constexpr bool value = sizeof(__test<_Tp>(0)) == 1; \
-    };                                                           \
-                                                                 \
-  template<typename _Tp>                                         \
-    struct __has_##_NTYPE                                        \
-    : integral_constant<bool, __has_##_NTYPE##_helper            \
-			<typename remove_cv<_Tp>::type>::value>  \
+#define _GLIBCXX_HAS_NESTED_TYPE(_NTYPE)				\
+  template<typename _Tp>						\
+    class __has_##_NTYPE##_helper					\
+    {									\
+      template<typename _Up>						\
+	struct _Wrap_type						\
+	{ };								\
+									\
+      template<typename _Up>						\
+	static true_type __test(_Wrap_type<typename _Up::_NTYPE>*);	\
+									\
+      template<typename _Up>						\
+	static false_type __test(...);					\
+									\
+    public:								\
+      typedef decltype(__test<_Tp>(0)) type;				\
+    };									\
+									\
+  template<typename _Tp>						\
+    struct __has_##_NTYPE						\
+    : public __has_##_NTYPE##_helper					\
+			<typename remove_cv<_Tp>::type>::type		\
     { };
 
 _GLIBCXX_END_NAMESPACE_VERSION
Index: testsuite/20_util/reference_wrapper/typedefs-3.cc
===================================================================
--- testsuite/20_util/reference_wrapper/typedefs-3.cc	(revision 197307)
+++ testsuite/20_util/reference_wrapper/typedefs-3.cc	(working copy)
@@ -44,7 +44,8 @@
 
 struct S012 : S0, S1, S2 { };
 
-using std::__sfinae_types;
+using std::true_type;
+using std::false_type;
 using std::integral_constant;
 using std::remove_cv;
 
Index: testsuite/20_util/make_signed/requirements/typedefs_neg.cc
===================================================================
--- testsuite/20_util/make_signed/requirements/typedefs_neg.cc	(revision 197307)
+++ testsuite/20_util/make_signed/requirements/typedefs_neg.cc	(working copy)
@@ -48,5 +48,5 @@
 // { dg-error "required from here" "" { target *-*-* } 40 }
 // { dg-error "required from here" "" { target *-*-* } 42 }
 
-// { dg-error "invalid use of incomplete type" "" { target *-*-* } 1613 }
-// { dg-error "declaration of" "" { target *-*-* } 1577 }
+// { dg-error "invalid use of incomplete type" "" { target *-*-* } 1597 }
+// { dg-error "declaration of" "" { target *-*-* } 1561 }
Index: testsuite/20_util/make_unsigned/requirements/typedefs_neg.cc
===================================================================
--- testsuite/20_util/make_unsigned/requirements/typedefs_neg.cc	(revision 197307)
+++ testsuite/20_util/make_unsigned/requirements/typedefs_neg.cc	(working copy)
@@ -48,5 +48,5 @@
 // { dg-error "required from here" "" { target *-*-* } 40 }
 // { dg-error "required from here" "" { target *-*-* } 42 }
 
-// { dg-error "invalid use of incomplete type" "" { target *-*-* } 1531 }
-// { dg-error "declaration of" "" { target *-*-* } 1495 }
+// { dg-error "invalid use of incomplete type" "" { target *-*-* } 1515 }
+// { dg-error "declaration of" "" { target *-*-* } 1479 }
Index: testsuite/20_util/declval/requirements/1_neg.cc
===================================================================
--- testsuite/20_util/declval/requirements/1_neg.cc	(revision 197307)
+++ testsuite/20_util/declval/requirements/1_neg.cc	(working copy)
@@ -19,7 +19,7 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// { dg-error "static assertion failed" "" { target *-*-* } 1871 }
+// { dg-error "static assertion failed" "" { target *-*-* } 1855 }
 
 #include <utility>
 
Index: testsuite/20_util/bind/ref_neg.cc
===================================================================
--- testsuite/20_util/bind/ref_neg.cc	(revision 197307)
+++ testsuite/20_util/bind/ref_neg.cc	(working copy)
@@ -30,10 +30,10 @@
 {
   const int dummy = 0;
   std::bind(&inc, _1)(0);               // { dg-error  "no match" }
-  // { dg-error "rvalue|const" "" { target *-*-* } 1347 }
-  // { dg-error "rvalue|const" "" { target *-*-* } 1361 }
-  // { dg-error "rvalue|const" "" { target *-*-* } 1375 }
-  // { dg-error "rvalue|const" "" { target *-*-* } 1389 }
+  // { dg-error "rvalue|const" "" { target *-*-* } 1315 }
+  // { dg-error "rvalue|const" "" { target *-*-* } 1329 }
+  // { dg-error "rvalue|const" "" { target *-*-* } 1343 }
+  // { dg-error "rvalue|const" "" { target *-*-* } 1357 }
   std::bind(&inc, std::ref(dummy))();	// { dg-error  "no match" }
 }
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]