public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""
@ 2014-05-14 13:39 Ed Smith-Rowland
  2014-05-14 13:59 ` Daniel Krügler
  0 siblings, 1 reply; 10+ messages in thread
From: Ed Smith-Rowland @ 2014-05-14 13:39 UTC (permalink / raw)
  To: gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 189 bytes --]

Make the machinery in bits/parse_number.h unsigned long long.
I had actually noticed this a while back but we were in stage 4. Then I 
forgot.. :-/

Built and tested on x84_64-linux.

OK?


[-- Attachment #2: CL_pr61166 --]
[-- Type: text/plain, Size: 253 bytes --]


2014-05-14  Ed Smith-Rowland  <3dw4rd@verizon.net>

	libstdc++/61166 overflow when parse number in std::duration operator""
	* include/bits/parse_numbers.h: Make the machinery unsigned long long.
	* testsuite/20_util/duration/literals/pr61166.cc: New.

[-- Attachment #3: patch_pr61166 --]
[-- Type: text/plain, Size: 13921 bytes --]

Index: include/bits/parse_numbers.h
===================================================================
--- include/bits/parse_numbers.h	(revision 210315)
+++ include/bits/parse_numbers.h	(working copy)
@@ -27,8 +27,8 @@
  *  Do not attempt to use it directly. @headername{chrono}
  */
 
-#ifndef _PARSE_NUMBERS_H
-#define _PARSE_NUMBERS_H 1
+#ifndef _GLIBCXX_PARSE_NUMBERS_H
+#define _GLIBCXX_PARSE_NUMBERS_H 1
 
 #pragma GCC system_header
 
@@ -36,262 +36,278 @@
 
 #if __cplusplus > 201103L
 
+#include <limits>
+
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
-namespace __parse_int {
+namespace __parse_int
+{
 
-  template<unsigned _Base, char _Dig>
+  template<unsigned long long _Base, char _Dig>
     struct _Digit;
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '0'>
     {
       static constexpr bool valid{true};
-      static constexpr unsigned value{0};
+      static constexpr unsigned long long value{0};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '1'>
     {
       static constexpr bool valid{true};
-      static constexpr unsigned value{1};
+      static constexpr unsigned long long value{1};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '2'>
     {
       static_assert(_Base > 2, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{2};
+      static constexpr unsigned long long value{2};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '3'>
     {
       static_assert(_Base > 3, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{3};
+      static constexpr unsigned long long value{3};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '4'>
     {
       static_assert(_Base > 4, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{4};
+      static constexpr unsigned long long value{4};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '5'>
     {
       static_assert(_Base > 5, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{5};
+      static constexpr unsigned long long value{5};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '6'>
     {
       static_assert(_Base > 6, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{6};
+      static constexpr unsigned long long value{6};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '7'>
     {
       static_assert(_Base > 7, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{7};
+      static constexpr unsigned long long value{7};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '8'>
     {
       static_assert(_Base > 8, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{8};
+      static constexpr unsigned long long value{8};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '9'>
     {
       static_assert(_Base > 9, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{9};
+      static constexpr unsigned long long value{9};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'a'>
     {
       static_assert(_Base > 0xa, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xa};
+      static constexpr unsigned long long value{0xa};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'A'>
     {
       static_assert(_Base > 0xa, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xa};
+      static constexpr unsigned long long value{0xa};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'b'>
     {
       static_assert(_Base > 0xb, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xb};
+      static constexpr unsigned long long value{0xb};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'B'>
     {
       static_assert(_Base > 0xb, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xb};
+      static constexpr unsigned long long value{0xb};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'c'>
     {
       static_assert(_Base > 0xc, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xc};
+      static constexpr unsigned long long value{0xc};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'C'>
     {
       static_assert(_Base > 0xc, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xc};
+      static constexpr unsigned long long value{0xc};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'd'>
     {
       static_assert(_Base > 0xd, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xd};
+      static constexpr unsigned long long value{0xd};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'D'>
     {
       static_assert(_Base > 0xd, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xd};
+      static constexpr unsigned long long value{0xd};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'e'>
     {
       static_assert(_Base > 0xe, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xe};
+      static constexpr unsigned long long value{0xe};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'E'>
     {
       static_assert(_Base > 0xe, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xe};
+      static constexpr unsigned long long value{0xe};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'f'>
     {
       static_assert(_Base > 0xf, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xf};
+      static constexpr unsigned long long value{0xf};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, 'F'>
     {
       static_assert(_Base > 0xf, "invalid digit");
       static constexpr bool valid{true};
-      static constexpr unsigned value{0xf};
+      static constexpr unsigned long long value{0xf};
     };
 
+  //  Radix
+  template<unsigned long long _Base>
+    struct _Digit<_Base, '.'>
+    {
+      static constexpr bool valid{false};
+      static constexpr unsigned long long value{0};
+    };
+
   //  Digit separator
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digit<_Base, '\''>
     {
       static constexpr bool valid{false};
-      static constexpr unsigned value{0};
+      static constexpr unsigned long long value{0};
     };
 
 
 //------------------------------------------------------------------------------
 
-  template<unsigned _Base, char _Dig, char... _Digs>
+  template<unsigned long long _Base, char _Dig, char... _Digs>
     struct _Digits_help
     {
-      static constexpr unsigned
+      static constexpr unsigned long long
 	value{_Digit<_Base, _Dig>::valid ?
 	      1U + _Digits_help<_Base, _Digs...>::value :
 	      _Digits_help<_Base, _Digs...>::value};
     };
 
-  template<unsigned _Base, char _Dig>
+  template<unsigned long long _Base, char _Dig>
     struct _Digits_help<_Base, _Dig>
     {
-      static constexpr unsigned value{_Digit<_Base, _Dig>::valid ? 1U : 0U};
+      static constexpr unsigned long long
+	value{_Digit<_Base, _Dig>::valid ? 1U : 0U};
     };
 
-  template<unsigned _Base, char... _Digs>
+  template<unsigned long long _Base, char... _Digs>
     struct _Digits
     {
-      static constexpr unsigned value{_Digits_help<_Base, _Digs...>::value};
+      static constexpr unsigned long long
+	value{_Digits_help<_Base, _Digs...>::value};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Digits<_Base>
     {
-      static constexpr unsigned value{0U};
+      static constexpr unsigned long long value{0U};
     };
 
 //------------------------------------------------------------------------------
 
-  template<unsigned _Base, char _Dig, char... _Digs>
+  template<unsigned long long _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};
+      static constexpr unsigned long long
+	value{_Digit<_Base, _Dig>::valid
+	    ? _Base * _Power_help<_Base, _Digs...>::value
+	    : _Power_help<_Base, _Digs...>::value};
     };
 
-  template<unsigned _Base, char _Dig>
+  template<unsigned long long _Base, char _Dig>
     struct _Power_help<_Base, _Dig>
     {
-      static constexpr unsigned value{_Digit<_Base, _Dig>::valid ? 1U : 0U};
+      static constexpr unsigned long long
+	value{_Digit<_Base, _Dig>::valid ? 1ULL : 0ULL};
     };
 
-  template<unsigned _Base, char... _Digs>
+  template<unsigned long long _Base, char... _Digs>
     struct _Power
     {
-      static constexpr unsigned value{_Power_help<_Base, _Digs...>::value};
+      static constexpr unsigned long long
+	value{_Power_help<_Base, _Digs...>::value};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Power<_Base>
     {
-      static constexpr unsigned value{0U};
+      static constexpr unsigned long long value{0ULL};
     };
 
 //------------------------------------------------------------------------------
 
-  template<unsigned _Base, unsigned _Pow, char _Dig, char... _Digs>
+  template<unsigned long long _Base, unsigned long long _Pow,
+	   char _Dig, char... _Digs>
     struct _Number_help
     {
-      static constexpr unsigned
+      static constexpr unsigned long long
 	value{_Digit<_Base, _Dig>::valid ?
 	      _Pow * _Digit<_Base, _Dig>::value
 	      + _Number_help<_Base, _Pow / _Base, _Digs...>::value :
@@ -298,26 +314,26 @@
 	      _Number_help<_Base, _Pow, _Digs...>::value};
     };
 
-  template<unsigned _Base, unsigned _Pow, char _Dig>
+  template<unsigned long long _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};
+      //static_assert(_Pow == 1ULL, "power should be one");
+      static constexpr unsigned long long
+	value{_Digit<_Base, _Dig>::valid ? _Digit<_Base, _Dig>::value : 0ULL};
     };
 
-  template<unsigned _Base, char... _Digs>
+  template<unsigned long long _Base, char... _Digs>
     struct _Number
     {
-      static constexpr unsigned
+      static constexpr unsigned long long
 	value{_Number_help<_Base, _Power<_Base, _Digs...>::value,
 			   _Digs...>::value};
     };
 
-  template<unsigned _Base>
+  template<unsigned long long _Base>
     struct _Number<_Base>
     {
-      static constexpr unsigned value{0U};
+      static constexpr unsigned long long value{0ULL};
     };
 
 //------------------------------------------------------------------------------
@@ -371,7 +387,8 @@
 } // namespace __parse_int
 
 
-namespace __select_int {
+namespace __select_int
+{
 
   template<unsigned long long _Val, typename... _Ints>
     struct _Select_int_base;
@@ -414,4 +431,4 @@
 
 #endif // __cplusplus > 201103L
 
-#endif // _PARSE_NUMBERS_H
+#endif // _GLIBCXX_PARSE_NUMBERS_H
Index: testsuite/20_util/duration/literals/pr61166.cc
===================================================================
--- testsuite/20_util/duration/literals/pr61166.cc	(revision 0)
+++ testsuite/20_util/duration/literals/pr61166.cc	(working copy)
@@ -0,0 +1,37 @@
+// { dg-do run }
+// { dg-options "-std=gnu++1y" }
+
+// Copyright (C) 2014 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <chrono>
+#include <testsuite_hooks.h>
+
+void
+test03()
+{
+  using namespace std::literals::chrono_literals;
+
+  // std::numeric_limits<unsigned>::max() == 4294967295
+  VERIFY( (429496729510h).count() == 429496729510 );
+}
+
+int
+main()
+{
+  test03();
+}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""
  2014-05-14 13:39 [PATCH, libstdc++/61166] overflow when parse number in std::duration operator"" Ed Smith-Rowland
@ 2014-05-14 13:59 ` Daniel Krügler
  2014-05-14 14:13   ` Jonathan Wakely
  2014-05-14 14:26   ` Ed Smith-Rowland
  0 siblings, 2 replies; 10+ messages in thread
From: Daniel Krügler @ 2014-05-14 13:59 UTC (permalink / raw)
  To: Ed Smith-Rowland; +Cc: gcc-patches, libstdc++

2014-05-14 15:38 GMT+02:00 Ed Smith-Rowland <3dw4rd@verizon.net>:
> Make the machinery in bits/parse_number.h unsigned long long.
> I had actually noticed this a while back but we were in stage 4. Then I
> forgot.. :-/
>
> Built and tested on x84_64-linux.
>
> OK?

I understand the reason why the corresponding static members value got
type unsigned long long, but why did the template parameter _Base also
require the same update?

Another question: Presumably "value" indded requires no uglification,
but what about the member "valid"? I would expect that this is no name
reserved by the library.

- Daniel



-- 

________________________________
SavedURI :Show URLShow URLSavedURI :
SavedURI :Hide URLHide URLSavedURI :
https://mail.google.com/_/scs/mail-static/_/js/k=gmail.main.de.LEt2fN4ilLE.O/m=m_i,t,it/am=OCMOBiHj9kJxhnelj6j997_NLil29vVAOBGeBBRgJwD-m_0_8B_AD-qOEw/rt=h/d=1/rs=AItRSTODy9wv1JKZMABIG3Ak8ViC4kuOWA?random=1395770800154https://mail.google.com/_/scs/mail-static/_/js/k=gmail.main.de.LEt2fN4ilLE.O/m=m_i,t,it/am=OCMOBiHj9kJxhnelj6j997_NLil29vVAOBGeBBRgJwD-m_0_8B_AD-qOEw/rt=h/d=1/rs=AItRSTODy9wv1JKZMABIG3Ak8ViC4kuOWA?random=1395770800154
________________________________

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""
  2014-05-14 13:59 ` Daniel Krügler
@ 2014-05-14 14:13   ` Jonathan Wakely
  2014-05-14 14:26   ` Ed Smith-Rowland
  1 sibling, 0 replies; 10+ messages in thread
From: Jonathan Wakely @ 2014-05-14 14:13 UTC (permalink / raw)
  To: Daniel Krügler; +Cc: Ed Smith-Rowland, gcc-patches, libstdc++

On 14 May 2014 14:59, Daniel Krügler wrote:
> 2014-05-14 15:38 GMT+02:00 Ed Smith-Rowland <3dw4rd@verizon.net>:
>> Make the machinery in bits/parse_number.h unsigned long long.
>> I had actually noticed this a while back but we were in stage 4. Then I
>> forgot.. :-/
>>
>> Built and tested on x84_64-linux.
>>
>> OK?
>
> I understand the reason why the corresponding static members value got
> type unsigned long long, but why did the template parameter _Base also
> require the same update?
>
> Another question: Presumably "value" indded requires no uglification,
> but what about the member "valid"? I would expect that this is no name
> reserved by the library.

Good point. (It's a member function in at least <future>, but we
shouldn't be reserving it elsewhere).

Any time I see a static const member of integral type called value I
wonder if the type should derive from a specialization of
std::integral_constant. You could probably also do:

using __valid = true_type;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""
  2014-05-14 13:59 ` Daniel Krügler
  2014-05-14 14:13   ` Jonathan Wakely
@ 2014-05-14 14:26   ` Ed Smith-Rowland
  2014-05-14 14:36     ` Jonathan Wakely
  1 sibling, 1 reply; 10+ messages in thread
From: Ed Smith-Rowland @ 2014-05-14 14:26 UTC (permalink / raw)
  To: Daniel Krügler; +Cc: gcc-patches, libstdc++

On 05/14/2014 09:59 AM, Daniel Krügler wrote:
> 2014-05-14 15:38 GMT+02:00 Ed Smith-Rowland <3dw4rd@verizon.net>:
>> Make the machinery in bits/parse_number.h unsigned long long.
>> I had actually noticed this a while back but we were in stage 4. Then I
>> forgot.. :-/
>>
>> Built and tested on x84_64-linux.
>>
>> OK?
> I understand the reason why the corresponding static members value got
> type unsigned long long, but why did the template parameter _Base also
> require the same update?
>
> Another question: Presumably "value" indded requires no uglification,
> but what about the member "valid"? I would expect that this is no name
> reserved by the library.
>
> - Daniel
>
>
>
You're right, _Base doesn't need that - only 2, 8, 10, 16 allowed (could 
do base64 someday, but still).  Change all got me.

I should uglify the valid members.
But in keeping with, say, our extension type traits and such maybe i 
should uglify value as well.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""
  2014-05-14 14:26   ` Ed Smith-Rowland
@ 2014-05-14 14:36     ` Jonathan Wakely
  2014-05-14 14:41       ` Jonathan Wakely
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2014-05-14 14:36 UTC (permalink / raw)
  To: Ed Smith-Rowland; +Cc: Daniel Krügler, gcc-patches, libstdc++

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""
  2014-05-14 14:36     ` Jonathan Wakely
@ 2014-05-14 14:41       ` Jonathan Wakely
  2014-05-14 15:36         ` Jonathan Wakely
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2014-05-14 14:41 UTC (permalink / raw)
  To: Ed Smith-Rowland; +Cc: Daniel Krügler, gcc-patches, libstdc++

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).

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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""
  2014-05-14 14:41       ` Jonathan Wakely
@ 2014-05-14 15:36         ` Jonathan Wakely
  2014-05-15 19:03           ` Jonathan Wakely
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2014-05-14 15:36 UTC (permalink / raw)
  To: Ed Smith-Rowland; +Cc: Daniel Krügler, gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]

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.)

[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 11414 bytes --]

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
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""
  2014-05-14 15:36         ` Jonathan Wakely
@ 2014-05-15 19:03           ` Jonathan Wakely
  2014-05-16  2:54             ` Ed Smith-Rowland
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2014-05-15 19:03 UTC (permalink / raw)
  To: Ed Smith-Rowland; +Cc: Daniel Krügler, gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 117 bytes --]

Here's a finished patch to simplify <bits/parse_numbers.h>

Tested x86_64-linux. Ed, any objection to this version?


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 15792 bytes --]

commit 87d26af2fc07f0c45a0a6676161ae1db1d7541b7
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed May 14 16:35:20 2014 +0100

    2014-05-15  Ed Smith-Rowland  <3dw4rd@verizon.net>
    	    Jonathan Wakely  <jwakely@redhat.com>
    
    	PR libstdc++/61166
    	* include/bits/parse_numbers.h: Use integral_constant to remove
    	duplication and simplify.
    	* testsuite/20_util/duration/literals/61166.cc: New.

diff --git a/libstdc++-v3/include/bits/parse_numbers.h b/libstdc++-v3/include/bits/parse_numbers.h
index 91a127c..0a42381a 100644
--- a/libstdc++-v3/include/bits/parse_numbers.h
+++ b/libstdc++-v3/include/bits/parse_numbers.h
@@ -27,8 +27,8 @@
  *  Do not attempt to use it directly. @headername{chrono}
  */
 
-#ifndef _PARSE_NUMBERS_H
-#define _PARSE_NUMBERS_H 1
+#ifndef _GLIBCXX_PARSE_NUMBERS_H
+#define _GLIBCXX_PARSE_NUMBERS_H 1
 
 #pragma GCC system_header
 
@@ -36,289 +36,181 @@
 
 #if __cplusplus > 201103L
 
+#include <limits>
+
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
-namespace __parse_int {
-
+namespace __parse_int
+{
   template<unsigned _Base, char _Dig>
     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>
+    { };
 
-  //  Digit separator
   template<unsigned _Base>
-    struct _Digit<_Base, '\''>
-    {
-      static constexpr bool valid{false};
-      static constexpr unsigned value{0};
-    };
-
-
-//------------------------------------------------------------------------------
-
-  template<unsigned _Base, char _Dig, char... _Digs>
-    struct _Digits_help
-    {
-      static constexpr unsigned
-	value{_Digit<_Base, _Dig>::valid ?
-	      1U + _Digits_help<_Base, _Digs...>::value :
-	      _Digits_help<_Base, _Digs...>::value};
-    };
-
-  template<unsigned _Base, char _Dig>
-    struct _Digits_help<_Base, _Dig>
-    {
-      static constexpr unsigned value{_Digit<_Base, _Dig>::valid ? 1U : 0U};
-    };
-
-  template<unsigned _Base, char... _Digs>
-    struct _Digits
-    {
-      static constexpr unsigned value{_Digits_help<_Base, _Digs...>::value};
-    };
+    struct _Digit<_Base, 'F'> : _Digit_impl<_Base, 0xf>
+    { };
 
+  //  Digit separator
   template<unsigned _Base>
-    struct _Digits<_Base>
+    struct _Digit<_Base, '\''> : integral_constant<unsigned, 0>
     {
-      static constexpr unsigned value{0U};
+      using __valid = false_type;
     };
 
 //------------------------------------------------------------------------------
 
+  template<unsigned long long _Val>
+    using __ull_constant = integral_constant<unsigned long long, _Val>;
+
   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 = typename _Power_help<_Base, _Digs...>::type;
+      using __valid_digit = typename _Digit<_Base, _Dig>::__valid;
+      using type
+	= __ull_constant<__next::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 = __ull_constant<__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> : __ull_constant<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 = __ull_constant<_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 type = __ull_constant<_Digit<_Base, _Dig>::value>;
     };
 
   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};
-    };
+    : __ull_constant<0>
+    { };
 
 //------------------------------------------------------------------------------
 //  This _Parse_int is the same 'level' as the old _Base_dispatch.
@@ -328,84 +220,62 @@ 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
 
 
-namespace __select_int {
-
+namespace __select_int
+{
   template<unsigned long long _Val, typename... _Ints>
     struct _Select_int_base;
 
   template<unsigned long long _Val, typename _IntType, typename... _Ints>
     struct _Select_int_base<_Val, _IntType, _Ints...>
-    : integral_constant
-      <
-	typename conditional
-	<
-	  _Val <= static_cast<unsigned long long>
-		    (std::numeric_limits<_IntType>::max()),
-	  _IntType,
-	  typename _Select_int_base<_Val, _Ints...>::value_type
-	>::type,
-	_Val
-      >
+    : conditional_t<(_Val <= std::numeric_limits<_IntType>::max()),
+		    integral_constant<_IntType, _Val>,
+		    _Select_int_base<_Val, _Ints...>>
     { };
 
   template<unsigned long long _Val>
-    struct _Select_int_base<_Val> : integral_constant<unsigned long long, _Val>
+    struct _Select_int_base<_Val>
     { };
 
   template<char... _Digs>
-    struct _Select_int
-    : _Select_int_base<
+    using _Select_int = typename _Select_int_base<
 	__parse_int::_Parse_int<_Digs...>::value,
 	unsigned char,
 	unsigned short,
 	unsigned int,
 	unsigned long,
 	unsigned long long
-      >
-    { };
+      >::type;
 
 } // namespace __select_int
 
@@ -414,4 +284,4 @@ _GLIBCXX_END_NAMESPACE_VERSION
 
 #endif // __cplusplus > 201103L
 
-#endif // _PARSE_NUMBERS_H
+#endif // _GLIBCXX_PARSE_NUMBERS_H
diff --git a/libstdc++-v3/testsuite/20_util/duration/literals/61166.cc b/libstdc++-v3/testsuite/20_util/duration/literals/61166.cc
new file mode 100644
index 0000000..e06adf8
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/duration/literals/61166.cc
@@ -0,0 +1,39 @@
+// { dg-do run }
+// { dg-options "-std=gnu++1y" }
+
+// Copyright (C) 2014 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// libstdc++/61166
+
+#include <chrono>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  using namespace std::literals::chrono_literals;
+
+  // std::numeric_limits<unsigned>::max() == 4294967295
+  VERIFY( (429496729510h).count() == 429496729510 );
+}
+
+int
+main()
+{
+  test01();
+}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""
  2014-05-15 19:03           ` Jonathan Wakely
@ 2014-05-16  2:54             ` Ed Smith-Rowland
  2014-05-16 11:09               ` Jonathan Wakely
  0 siblings, 1 reply; 10+ messages in thread
From: Ed Smith-Rowland @ 2014-05-16  2:54 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Daniel Krügler, gcc-patches, libstdc++

On 05/15/2014 03:03 PM, Jonathan Wakely wrote:
> Here's a finished patch to simplify <bits/parse_numbers.h>
>
> Tested x86_64-linux. Ed, any objection to this version?
>
This looks great, thanks!

Having done that should we actually stop using it as suggested in the 
bug trail? ;-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator""
  2014-05-16  2:54             ` Ed Smith-Rowland
@ 2014-05-16 11:09               ` Jonathan Wakely
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Wakely @ 2014-05-16 11:09 UTC (permalink / raw)
  To: Ed Smith-Rowland; +Cc: Daniel Krügler, gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 1751 bytes --]

On 15/05/14 22:54 -0400, Ed Smith-Rowland wrote:
>On 05/15/2014 03:03 PM, Jonathan Wakely wrote:
>>Here's a finished patch to simplify <bits/parse_numbers.h>
>>
>>Tested x86_64-linux. Ed, any objection to this version?
>>
>This looks great, thanks!

I committed that to trunk, I'll put it on the 4.9 branch too.

>Having done that should we actually stop using it as suggested in the 
>bug trail? ;-)

I was going to do that, then realised that there's a defect in the
standard where it requires overflow in duration integer literals to be
diagnosed. That's only possible with literal operator templates, so I
think we should keep your _Parse_int code, but apply the attached
change to detect overflow.

As the TODO comment says, it should be sufficient to simply
instantiate integral_constant<_Rep, _Val::value> to give a diagnostic
when _Rep{_Value::value} is narrowing, but GCC only gives a warning
for it, and that's suppressed in a system header, so I do an explicit
static_assert.  That could be replaced with ...

#pragma GCC diagnostic push
#pragma GCC diagnostic error "-Woverflow"
#pragma GCC diagnostic error "-Wsystem-headers"
    template<typename _Dur, char... _Digits>
      constexpr _Dur __check_overflow()
      {
        using _Val = __parse_int::_Parse_int<_Digits...>;
        using _Rep = typename _Dur::rep;
        return _Dur{integral_constant<_Rep, _Val::value>::value};
      }
#pragma GCC diagnostic pop

... but I have plans to do that sort of thing more widely, which I'll
deal with another time as part of https://gcc.gnu.org/PR50871 and/or
https://gcc.gnu.org/PR58876 (what do other people think about using
diagnostic pragmas to locally re-enable diagnostics in our headers?)

Tested x86_64-linux, committed to trunk.

[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 7717 bytes --]

commit 361c9b79e0b1c7f2435f5b0b369a139b216dee90
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 16 10:31:38 2014 +0100

    	* include/bits/parse_numbers.h (__parse_int::_Number_help): Check for
    	overflow.
    	* include/std/chrono (chrono_literals::__select_type::_Select_type):
    	Remove.
    	(chrono_literals::_Checked_integral_constant): Define.
    	Simplify UDL operator templates and check for overflow.
    	* testsuite/20_util/duration/literals/range.cc: New.

diff --git a/libstdc++-v3/include/bits/parse_numbers.h b/libstdc++-v3/include/bits/parse_numbers.h
index 0a42381a..a29d127 100644
--- a/libstdc++-v3/include/bits/parse_numbers.h
+++ b/libstdc++-v3/include/bits/parse_numbers.h
@@ -193,6 +193,7 @@ namespace __parse_int
 				  _Pow / (_Base * __valid_digit::value),
 				  _Digs...>;
       using type = __ull_constant<_Pow * __digit::value + __next::type::value>;
+      static_assert((type::value / _Pow) == __digit::value, "overflow");
     };
 
   template<unsigned _Base, unsigned long long _Pow, char _Dig>
diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index b114e02..39ad5e3 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -787,117 +787,79 @@ _GLIBCXX_END_NAMESPACE_VERSION
   inline namespace chrono_literals
   {
 
-    namespace __select_type
-    {
-
-      using namespace __parse_int;
-
-      template<unsigned long long _Val, typename _Dur>
-	struct _Select_type
-	: conditional<
-	    _Val <= static_cast<unsigned long long>
-		      (numeric_limits<typename _Dur::rep>::max()),
-	    _Dur, void>
-	{
-	  static constexpr typename _Select_type::type
-	    value{static_cast<typename _Select_type::type>(_Val)};
-	};
-
-      template<unsigned long long _Val, typename _Dur>
-	constexpr typename _Select_type<_Val, _Dur>::type
-	_Select_type<_Val, _Dur>::value;
+    template<typename _Rep, unsigned long long _Val>
+      struct _Checked_integral_constant
+      : integral_constant<_Rep, static_cast<_Rep>(_Val)>
+      {
+	static_assert(_Checked_integral_constant::value > 0
+		      && _Checked_integral_constant::value == _Val,
+		      "literal value cannot be represented by duration type");
+      };
 
-    } // __select_type
+    template<typename _Dur, char... _Digits>
+      constexpr _Dur __check_overflow()
+      {
+	using _Val = __parse_int::_Parse_int<_Digits...>;
+	using _Rep = typename _Dur::rep;
+	// TODO: should be simply integral_constant<_Rep, _Val::value>
+	// but GCC doesn't reject narrowing conversions to _Rep.
+	using _CheckedVal = _Checked_integral_constant<_Rep, _Val::value>;
+	return _Dur{_CheckedVal::value};
+      }
 
     constexpr chrono::duration<long double, ratio<3600,1>>
     operator""h(long double __hours)
     { return chrono::duration<long double, ratio<3600,1>>{__hours}; }
 
     template <char... _Digits>
-      constexpr typename
-      __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value,
-			     chrono::hours>::type
+      constexpr chrono::hours
       operator""h()
-      {
-	return __select_type::_Select_type<
-			  __select_int::_Select_int<_Digits...>::value,
-			  chrono::hours>::value;
-      }
+      { return __check_overflow<chrono::hours, _Digits...>(); }
 
     constexpr chrono::duration<long double, ratio<60,1>>
     operator""min(long double __mins)
     { return chrono::duration<long double, ratio<60,1>>{__mins}; }
 
     template <char... _Digits>
-      constexpr typename
-      __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value,
-			     chrono::minutes>::type
+      constexpr chrono::minutes
       operator""min()
-      {
-	return __select_type::_Select_type<
-			  __select_int::_Select_int<_Digits...>::value,
-			  chrono::minutes>::value;
-      }
+      { return __check_overflow<chrono::minutes, _Digits...>(); }
 
     constexpr chrono::duration<long double>
     operator""s(long double __secs)
     { return chrono::duration<long double>{__secs}; }
 
     template <char... _Digits>
-      constexpr typename
-      __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value,
-			     chrono::seconds>::type
+      constexpr chrono::seconds
       operator""s()
-      {
-	return __select_type::_Select_type<
-			  __select_int::_Select_int<_Digits...>::value,
-			  chrono::seconds>::value;
-      }
+      { return __check_overflow<chrono::seconds, _Digits...>(); }
 
     constexpr chrono::duration<long double, milli>
     operator""ms(long double __msecs)
     { return chrono::duration<long double, milli>{__msecs}; }
 
     template <char... _Digits>
-      constexpr typename
-      __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value,
-			     chrono::milliseconds>::type
+      constexpr chrono::milliseconds
       operator""ms()
-      {
-	return __select_type::_Select_type<
-			  __select_int::_Select_int<_Digits...>::value,
-			  chrono::milliseconds>::value;
-      }
+      { return __check_overflow<chrono::milliseconds, _Digits...>(); }
 
     constexpr chrono::duration<long double, micro>
     operator""us(long double __usecs)
     { return chrono::duration<long double, micro>{__usecs}; }
 
     template <char... _Digits>
-      constexpr typename
-      __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value,
-			     chrono::microseconds>::type
+      constexpr chrono::microseconds
       operator""us()
-      {
-	return __select_type::_Select_type<
-			  __select_int::_Select_int<_Digits...>::value,
-			  chrono::microseconds>::value;
-      }
+      { return __check_overflow<chrono::microseconds, _Digits...>(); }
 
     constexpr chrono::duration<long double, nano>
     operator""ns(long double __nsecs)
     { return chrono::duration<long double, nano>{__nsecs}; }
 
     template <char... _Digits>
-      constexpr typename
-      __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value,
-			     chrono::nanoseconds>::type
+      constexpr chrono::nanoseconds
       operator""ns()
-      {
-	return __select_type::_Select_type<
-			  __select_int::_Select_int<_Digits...>::value,
-			  chrono::nanoseconds>::value;
-      }
+      { return __check_overflow<chrono::nanoseconds, _Digits...>(); }
 
   } // inline namespace chrono_literals
   } // inline namespace literals
diff --git a/libstdc++-v3/testsuite/20_util/duration/literals/range.cc b/libstdc++-v3/testsuite/20_util/duration/literals/range.cc
new file mode 100644
index 0000000..c005df6
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/duration/literals/range.cc
@@ -0,0 +1,31 @@
+// { dg-do compile }
+// { dg-options "-std=gnu++1y" }
+
+// Copyright (C) 2014 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <chrono>
+
+void
+test01()
+{
+  using namespace std::literals::chrono_literals;
+
+  // std::numeric_limits<int64_t>::max() == 9223372036854775807;
+  auto h = 9223372036854775808h;
+  // { dg-error "cannot be represented" "" { target *-*-* } 794 }
+}

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-05-16 11:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-14 13:39 [PATCH, libstdc++/61166] overflow when parse number in std::duration operator"" Ed Smith-Rowland
2014-05-14 13:59 ` Daniel Krügler
2014-05-14 14:13   ` Jonathan Wakely
2014-05-14 14:26   ` Ed Smith-Rowland
2014-05-14 14:36     ` Jonathan Wakely
2014-05-14 14:41       ` Jonathan Wakely
2014-05-14 15:36         ` Jonathan Wakely
2014-05-15 19:03           ` Jonathan Wakely
2014-05-16  2:54             ` Ed Smith-Rowland
2014-05-16 11:09               ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).