This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ 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: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""


On 14/05/14 15:41 +0100, Jonathan Wakely wrote:
On 14 May 2014 15:36, Jonathan Wakely wrote:
On 14 May 2014 15:25, Ed Smith-Rowland wrote:
But in keeping with, say, our extension type traits and such maybe i should
uglify value as well.

No, just derive from std::integral_constant and you get value "for free".

You already use integral_constant in that file, so the name "value" is
already used.

That also has the advantage that _Digit<B, 'f'> and _Digit<B, 'F'>
share the same base class, so you don't end up with two different
static members with the same value (and if you make __valid a typedef
as I suggested you don't have any static members for that either).

I've attached a first pass at doing it that way, using
integral_constant base classes and working with types not values as
much as possible. This gets rid of all static const members except in
_Digits<> (where is that used?)

Do we really need _Digit::value to be unsigned long long, or is it
only the results in _Power_help and _Number_help that need to be
64-bit?

Because I made _Digit derive from integral_constant<unsigned> that
means the partial specialization of _Number_help for single digits has
_Number_help::type as integral_constant<unsigned>, where the other
specializations have integral_constant<unsigned long long>. That's
inconsistent, but works. Maybe we'd want to be consistent though, and
use this instead of typename _Digit<Base, _Dig>::type:

 using type = integral_constant<unsigned long long,
                                _Digit<_Base, _Dig>::value>;

(Also, I wonder if we could use std::ratio for the power/base
arithmetic, but it might not help.)
diff --git a/libstdc++-v3/include/bits/parse_numbers.h b/libstdc++-v3/include/bits/parse_numbers.h
index 91a127c..055058e 100644
--- a/libstdc++-v3/include/bits/parse_numbers.h
+++ b/libstdc++-v3/include/bits/parse_numbers.h
@@ -46,185 +46,109 @@ namespace __parse_int {
     struct _Digit;
 
   template<unsigned _Base>
-    struct _Digit<_Base, '0'>
+    struct _Digit<_Base, '0'> : integral_constant<unsigned, 0>
     {
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0};
+      using __valid = true_type;
     };
 
   template<unsigned _Base>
-    struct _Digit<_Base, '1'>
+    struct _Digit<_Base, '1'> : integral_constant<unsigned, 1>
     {
-      static constexpr bool valid{true};
-      static constexpr unsigned value{1};
+      using __valid = true_type;
     };
 
-  template<unsigned _Base>
-    struct _Digit<_Base, '2'>
+  template<unsigned _Base, unsigned _Val>
+    struct _Digit_impl : integral_constant<unsigned, _Val>
     {
-      static_assert(_Base > 2, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{2};
+      static_assert(_Base > _Val, "invalid digit");
+      using __valid = true_type;
     };
 
   template<unsigned _Base>
-    struct _Digit<_Base, '3'>
-    {
-      static_assert(_Base > 3, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{3};
-    };
+    struct _Digit<_Base, '2'> : _Digit_impl<_Base, 2>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, '4'>
-    {
-      static_assert(_Base > 4, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{4};
-    };
+    struct _Digit<_Base, '3'> : _Digit_impl<_Base, 3>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, '5'>
-    {
-      static_assert(_Base > 5, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{5};
-    };
+    struct _Digit<_Base, '4'> : _Digit_impl<_Base, 4>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, '6'>
-    {
-      static_assert(_Base > 6, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{6};
-    };
+    struct _Digit<_Base, '5'> : _Digit_impl<_Base, 5>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, '7'>
-    {
-      static_assert(_Base > 7, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{7};
-    };
+    struct _Digit<_Base, '6'> : _Digit_impl<_Base, 6>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, '8'>
-    {
-      static_assert(_Base > 8, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{8};
-    };
+    struct _Digit<_Base, '7'> : _Digit_impl<_Base, 7>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, '9'>
-    {
-      static_assert(_Base > 9, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{9};
-    };
+    struct _Digit<_Base, '8'> : _Digit_impl<_Base, 8>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'a'>
-    {
-      static_assert(_Base > 0xa, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xa};
-    };
+    struct _Digit<_Base, '9'> : _Digit_impl<_Base, 9>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'A'>
-    {
-      static_assert(_Base > 0xa, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xa};
-    };
+    struct _Digit<_Base, 'a'> : _Digit_impl<_Base, 0xa>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'b'>
-    {
-      static_assert(_Base > 0xb, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xb};
-    };
+    struct _Digit<_Base, 'A'> : _Digit_impl<_Base, 0xa>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'B'>
-    {
-      static_assert(_Base > 0xb, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xb};
-    };
+    struct _Digit<_Base, 'b'> : _Digit_impl<_Base, 0xb>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'c'>
-    {
-      static_assert(_Base > 0xc, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xc};
-    };
+    struct _Digit<_Base, 'B'> : _Digit_impl<_Base, 0xb>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'C'>
-    {
-      static_assert(_Base > 0xc, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xc};
-    };
+    struct _Digit<_Base, 'c'> : _Digit_impl<_Base, 0xc>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'd'>
-    {
-      static_assert(_Base > 0xd, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xd};
-    };
+    struct _Digit<_Base, 'C'> : _Digit_impl<_Base, 0xc>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'D'>
-    {
-      static_assert(_Base > 0xd, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xd};
-    };
+    struct _Digit<_Base, 'd'> : _Digit_impl<_Base, 0xd>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'e'>
-    {
-      static_assert(_Base > 0xe, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xe};
-    };
+    struct _Digit<_Base, 'D'> : _Digit_impl<_Base, 0xd>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'E'>
-    {
-      static_assert(_Base > 0xe, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xe};
-    };
+    struct _Digit<_Base, 'e'> : _Digit_impl<_Base, 0xe>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'f'>
-    {
-      static_assert(_Base > 0xf, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xf};
-    };
+    struct _Digit<_Base, 'E'> : _Digit_impl<_Base, 0xe>
+    { };
 
   template<unsigned _Base>
-    struct _Digit<_Base, 'F'>
-    {
-      static_assert(_Base > 0xf, "invalid digit");
-      static constexpr bool valid{true};
-      static constexpr unsigned value{0xf};
-    };
+    struct _Digit<_Base, 'f'> : _Digit_impl<_Base, 0xf>
+    { };
+
+  template<unsigned _Base>
+    struct _Digit<_Base, 'F'> : _Digit_impl<_Base, 0xf>
+    { };
 
   //  Digit separator
   template<unsigned _Base>
-    struct _Digit<_Base, '\''>
+    struct _Digit<_Base, '\''> : integral_constant<unsigned, 0>
     {
-      static constexpr bool valid{false};
-      static constexpr unsigned value{0};
+      using __valid = false_type;
     };
 
 
@@ -262,63 +186,59 @@ namespace __parse_int {
   template<unsigned _Base, char _Dig, char... _Digs>
     struct _Power_help
     {
-      static constexpr unsigned
-	value{_Digit<_Base, _Dig>::valid ?
-	      _Base * _Power_help<_Base, _Digs...>::value :
-	      _Power_help<_Base, _Digs...>::value};
+      using __next = _Power_help<_Base, _Digs...>;
+      using __valid_digit = typename _Digit<_Base, _Dig>::__valid;
+      using type = integral_constant<unsigned long long,
+	    __next::type::value * (__valid_digit{} ? _Base : 1ULL)>;
     };
 
   template<unsigned _Base, char _Dig>
     struct _Power_help<_Base, _Dig>
     {
-      static constexpr unsigned value{_Digit<_Base, _Dig>::valid ? 1U : 0U};
+      using __valid_digit = typename _Digit<_Base, _Dig>::__valid;
+      using type = integral_constant<unsigned long long, __valid_digit::value>;
     };
 
   template<unsigned _Base, char... _Digs>
-    struct _Power
-    {
-      static constexpr unsigned value{_Power_help<_Base, _Digs...>::value};
-    };
+    struct _Power : _Power_help<_Base, _Digs...>::type
+    { };
 
   template<unsigned _Base>
-    struct _Power<_Base>
-    {
-      static constexpr unsigned value{0U};
-    };
+    struct _Power<_Base> : integral_constant<unsigned long long, 0>
+    { };
 
 //------------------------------------------------------------------------------
 
-  template<unsigned _Base, unsigned _Pow, char _Dig, char... _Digs>
+  template<unsigned _Base, unsigned long long _Pow, char _Dig, char... _Digs>
     struct _Number_help
     {
-      static constexpr unsigned
-	value{_Digit<_Base, _Dig>::valid ?
-	      _Pow * _Digit<_Base, _Dig>::value
-	      + _Number_help<_Base, _Pow / _Base, _Digs...>::value :
-	      _Number_help<_Base, _Pow, _Digs...>::value};
+      using __digit = _Digit<_Base, _Dig>;
+      using __valid_digit = typename __digit::__valid;
+      using __next = _Number_help<_Base,
+				  _Pow / (_Base * __valid_digit::value),
+				  _Digs...>;
+      using type
+	= integral_constant<unsigned long long,
+			    _Pow * __digit::value + __next::type::value>;
     };
 
-  template<unsigned _Base, unsigned _Pow, char _Dig>
+  template<unsigned _Base, unsigned long long _Pow, char _Dig>
     struct _Number_help<_Base, _Pow, _Dig>
     {
       //static_assert(_Pow == 1U, "power should be one");
-      static constexpr unsigned
-	value{_Digit<_Base, _Dig>::valid ? _Digit<_Base, _Dig>::value : 0U};
+      using __digit = _Digit<_Base, _Dig>;
+      using type = typename _Digit<_Base, _Dig>::type;
     };
 
   template<unsigned _Base, char... _Digs>
     struct _Number
-    {
-      static constexpr unsigned
-	value{_Number_help<_Base, _Power<_Base, _Digs...>::value,
-			   _Digs...>::value};
-    };
+    : _Number_help<_Base, _Power<_Base, _Digs...>::value, _Digs...>::type
+    { };
 
   template<unsigned _Base>
     struct _Number<_Base>
-    {
-      static constexpr unsigned value{0U};
-    };
+    : integral_constant<unsigned long long, 0>
+    { };
 
 //------------------------------------------------------------------------------
 //  This _Parse_int is the same 'level' as the old _Base_dispatch.
@@ -328,45 +248,33 @@ namespace __parse_int {
 
   template<char... _Digs>
     struct _Parse_int<'0', 'b', _Digs...>
-    {
-      static constexpr unsigned long long
-	value{_Number<2U, _Digs...>::value};
-    };
+    : _Number<2U, _Digs...>::type
+    { };
 
   template<char... _Digs>
     struct _Parse_int<'0', 'B', _Digs...>
-    {
-      static constexpr unsigned long long
-	value{_Number<2U, _Digs...>::value};
-    };
+    : _Number<2U, _Digs...>::type
+    { };
 
   template<char... _Digs>
     struct _Parse_int<'0', 'x', _Digs...>
-    {
-      static constexpr unsigned long long
-	value{_Number<16U, _Digs...>::value};
-    };
+    : _Number<16U, _Digs...>::type
+    { };
 
   template<char... _Digs>
     struct _Parse_int<'0', 'X', _Digs...>
-    {
-      static constexpr unsigned long long
-	value{_Number<16U, _Digs...>::value};
-    };
+    : _Number<16U, _Digs...>::type
+    { };
 
   template<char... _Digs>
     struct _Parse_int<'0', _Digs...>
-    {
-      static constexpr unsigned long long
-	value{_Number<8U, _Digs...>::value};
-    };
+    : _Number<8U, _Digs...>::type
+    { };
 
   template<char... _Digs>
     struct _Parse_int
-    {
-      static constexpr unsigned long long
-	value{_Number<10U, _Digs...>::value};
-    };
+    : _Number<10U, _Digs...>::type
+    { };
 
 } // namespace __parse_int
 

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