[Bug c++/69905] Digit separators break literal operators
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69905
Jonathan Wakely changed:
What|Removed |Added
Status|NEW |RESOLVED
Resolution|--- |DUPLICATE
--- Comment #9 from Jonathan Wakely ---
(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
> int main() {
> using std::chrono_literals;
> auto time = operator""s(0x8000'''''ULL);
> }
>
> 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 ***
[Bug c++/69905] Digit separators break literal operators
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69905
--- Comment #8 from Jonathan Wakely ---
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
int main() {
using namespace std::chrono_literals;
auto time = operator""s(0x8000'''''ULL);
static_assert(std::is_same(), "");
}
[Bug c++/69905] Digit separators break literal operators
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69905
andreas.molzer at gmx dot de changed:
What|Removed |Added
CC||andreas.molzer at gmx dot de
--- Comment #7 from andreas.molzer at gmx dot de ---
Leaving out the overload for operator""s(long double) will silently break valid
code. Consider:
#include
int main() {
using std::chrono_literals;
auto time = operator""s(0x8000'''''ULL);
}
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.
[Bug c++/69905] Digit separators break literal operators
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69905 emsr at gcc dot gnu.org changed: What|Removed |Added CC||emsr at gcc dot gnu.org --- Comment #6 from emsr at gcc dot gnu.org --- 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
[Bug c++/69905] Digit separators break literal operators
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69905 Jonathan Wakely changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2016-10-26 Ever confirmed|0 |1 --- Comment #5 from Jonathan Wakely --- 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
[Bug c++/69905] Digit separators break literal operators
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69905
TC changed:
What|Removed |Added
CC||rs2740 at gmail dot com
--- Comment #4 from TC ---
> template
> 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");
[Bug c++/69905] Digit separators break literal operators
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69905
aaron.mcdaid at gmail dot com changed:
What|Removed |Added
CC||aaron.mcdaid at gmail dot com
--- Comment #3 from aaron.mcdaid at gmail dot com ---
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
constexpr chrono::seconds
operator""s()
{ return __check_overflow();
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!)
[Bug c++/69905] Digit separators break literal operators
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69905
Tad Ashlock changed:
What|Removed |Added
CC||tad.ashlock at gmail dot com
--- Comment #2 from Tad Ashlock ---
I can confirm this problem, and add a little more detail. Here's the code I
used:
#include
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; 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]’
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,
^
[Bug c++/69905] Digit separators break literal operators
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69905 Marek Polacek changed: What|Removed |Added CC||mpolacek at gcc dot gnu.org --- Comment #1 from Marek Polacek --- It compiles for me with GCC 6 (and gnu++14).
