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