The following code fails to compile when the macro is defined: #include <chrono> using namespace std::literals; #if defined SHOW_BUG auto x = 1'23s #else auto x = 123s; #endif I /think/ this is a problem in the front-end, rather than the library, as the digit separator should not be visible to the library call.
It compiles for me with GCC 6 (and gnu++14).
I can confirm this problem, and add a little more detail. Here's the code I used: #include <chrono> using namespace std::chrono_literals; int main () { auto const test1 = 1100s; auto const test2 = 1'000s; auto const test3 = 1'100s; return 0; } The test1 and test2 assignments compile, indicating that 1100 seconds isn't too large of a number, and 1'000 is accepted. It turns out that a non-0 digit following the separator is causing the problem. But test3 fails with the message: g++ -std=c++14 test.cc In file included from /opt/tools/gcc/6.1.0/include/c++/6.1.0/chrono:42:0, from test.cc:1: /opt/tools/gcc/6.1.0/include/c++/6.1.0/bits/parse_numbers.h: In instantiation of ‘struct std::__parse_int::_Number_help<10u, 100ull, '\'', '1', '0', '0'>’: /opt/tools/gcc/6.1.0/include/c++/6.1.0/bits/parse_numbers.h:195:57: required from ‘struct std::__parse_int::_Number_help<10u, 1000ull, '1', '\'', '1', '0', '0'>’ /opt/tools/gcc/6.1.0/include/c++/6.1.0/bits/parse_numbers.h:208:12: required from ‘struct std::__parse_int::_Number<10u, '1', '\'', '1', '0', '0'>’ /opt/tools/gcc/6.1.0/include/c++/6.1.0/bits/parse_numbers.h:248:12: required from ‘struct std::__parse_int::_Parse_int<'1', '\'', '1', '0', '0'>’ /opt/tools/gcc/6.1.0/include/c++/6.1.0/chrono:808:67: required from ‘constexpr _Dur std::literals::chrono_literals::__check_overflow() [with _Dur = std::chrono::duration<long int>; char ..._Digits = {'1', '\'', '1', '0', '0'}]’ /opt/tools/gcc/6.1.0/include/c++/6.1.0/chrono:837:61: required from ‘constexpr std::chrono::seconds std::literals::chrono_literals::operator""s() [with char ..._Digits = {'1', '\'', '1', '0', '0'}; std::chrono::seconds = std::chrono::duration<long int>]’ test.cc:7:24: required from here /opt/tools/gcc/6.1.0/include/c++/6.1.0/bits/parse_numbers.h:196:7: error: static assertion failed: integer literal does not fit in unsigned long long static_assert((type::value / _Pow) == __digit::value, ^~~~~~~~~~~~~
I think this could be fixed by defining the missing overload of `operator""s` for `unsigned long long`. As I understand it, the standard requires two overloads, `long double` and `unsigned long long`. However, only the latter is present in gcc-6.2.0. The `chrono` file is missing `unsigned unsigned long`, but has the following instead: template <char... _Digits> constexpr chrono::seconds operator""s() { return __check_overflow<chrono::seconds, _Digits...>(); This appears to be failing to interpret the separator correctly. (Apologies if I have misinterpreted, this is my first time looking in the chrono header!)
> template<unsigned _Base, unsigned long long _Pow, char _Dig, char... _Digs> > struct _Number_help > { > using __digit = _Digit<_Base, _Dig>; > using __valid_digit = typename __digit::__valid; > using __next = _Number_help<_Base, > __valid_digit::value ? _Pow / _Base : _Pow, > _Digs...>; > using type = __ull_constant<_Pow * __digit::value + __next::type::value>; > static_assert((type::value / _Pow) == __digit::value, > "integer literal does not fit in unsigned long long"); > }; The issue is that when __valid_digit is false_type (i.e., we have a digit separator), the static_assert check should not be done. In this case, __next is computed using the same _Pow, and __digit::value is zero, which means that it currently fires whenever the next digit is nonzero. I think the fix is simply static_assert(!__valid_digit::value || (type::value / _Pow) == __digit::value, "integer literal does not fit in unsigned long long");
Yes that looks right, we didn't consider digit separators in that function template. It's by design that we use that rather than an operator""s(unsigned long long) because otherwise it's not possible to check for overflow, see https://wg21.link/lwg2383
If no one beats me to it I'll post a patch in an hour after testing. I knew I added separators and I thought I had tested it... Ed
Leaving out the overload for operator""s(long double) will silently break valid code. Consider: #include <chrono> int main() { using std::chrono_literals; auto time = operator""s(0x8000'0000'0000'0000'0000'0000ULL); } According to standard, this should select the overload with unsigned long long argument, which does not exist. It will instead silently perform a numeric conversion to long double, resulting in loss of accuracy on targets where gcc does not use extended 80-bit double. Any other integer arguments, which should lead to overload resolution failure, will also be double durations albeit with less immediately catastrophic results. However, that doesn't mean the long term effects are no issue. Consider that the type of time.count() also unexpectedly resolves as long double. Or even worse that assigning decltype(time)::max() to unsigned long long is undefined behaviour. It might be more fitting to open the missing declarations as a separately tracked bug, improving visibility of the standard issue as well.
The testcase in comment 2 (the only one in the bug that's actually correct!) works now, fixed by r258157 for Bug 84671. The problem in comment 7 still exists, the corrected testcase is: #include <chrono> int main() { using namespace std::chrono_literals; auto time = operator""s(0x8000'0000'0000'0000'0000'0000ULL); static_assert(std::is_same<decltype(time)::rep, unsigned long long>(), ""); }
(In reply to andreas.molzer from comment #7) > Leaving out the overload for operator""s(long double) will silently break We provide that overload though. What do you mean? > valid code. Consider: > > #include <chrono> > int main() { > using std::chrono_literals; > auto time = operator""s(0x8000'0000'0000'0000'0000'0000ULL); > } > > According to standard, this should select the overload with unsigned long > long argument, which does not exist. No, according to the standard this is ill-formed because the value doesn't fit in an unsigned long long (assuming 64 bits). GCC accepts it unless you use -pedantic-errors, but this isn't valid code. > It will instead silently perform a > numeric conversion to long double, resulting in loss of accuracy on targets > where gcc does not use extended 80-bit double. Any other integer arguments, > which should lead to overload resolution failure, will also be double > durations albeit with less immediately catastrophic results. Again, this doesn't silently break valid code, because what you're doing should be ill-formed according to the standard. So not valid code. > However, that doesn't mean the long term effects are no issue. Consider > that the type of time.count() also unexpectedly resolves as long double. Or > even worse that assigning decltype(time)::max() to unsigned long long is > undefined behaviour. > > It might be more fitting to open the missing declarations as a separately > tracked bug, improving visibility of the standard issue as well. Feel free to open a new bug (but this is not the place to report problems in the standard, see https://cplusplus.github.io/LWG/lwg-active.html#submit_issue for that). I'm going to close this one as a dup of Bug 84671 because I don't think there's any compiler bug here. The original problem was due to the library not handling digit separators in its UDL operators, which is fixed. *** This bug has been marked as a duplicate of bug 84671 ***