| Message ID | YJKI/p2/eHjthgpe@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [committed] libstdc++: Use unsigned char argument to std::isdigit | expand |
On 05/05/21 2:01 pm, Jonathan Wakely via Libstdc++ wrote: > Passing plain char to isdigit is undefined if the value is negative. > > libstdc++-v3/ChangeLog: > > * include/std/charconv (__from_chars_alnum): Pass unsigned > char to std::isdigit. > > Tested powerpc64le-linux. Committed to trunk. > unsigned char __c = *__first; - if (std::isdigit(__c)) + if (std::isdigit(static_cast<unsigned char>(__c))) I am very curious to know what this static_cast<unsigned char> does on __c which is already unsigned char ? If it does I'll just start to hate C++ :-) Maybe you wanted to put it on the previous *__first ?
On 05/05/21 21:57 +0200, François Dumont via Libstdc++ wrote: >On 05/05/21 2:01 pm, Jonathan Wakely via Libstdc++ wrote: >>Passing plain char to isdigit is undefined if the value is negative. >> >>libstdc++-v3/ChangeLog: >> >> * include/std/charconv (__from_chars_alnum): Pass unsigned >> char to std::isdigit. >> >>Tested powerpc64le-linux. Committed to trunk. >> > unsigned char __c = *__first; >- if (std::isdigit(__c)) >+ if (std::isdigit(static_cast<unsigned char>(__c))) > >I am very curious to know what this static_cast<unsigned char> does on >__c which is already unsigned char ? If it does I'll just start to >hate C++ :-) > >Maybe you wanted to put it on the previous *__first ? Ugh, yes, but it's not even needed there because the implicit conversion is fine. We do need to fix the isspace calls in src/c++11/debug.cc but this one was already correct. Thanks!
On 05/05/21 22:14 +0100, Jonathan Wakely wrote: >On 05/05/21 21:57 +0200, François Dumont via Libstdc++ wrote: >>On 05/05/21 2:01 pm, Jonathan Wakely via Libstdc++ wrote: >>>Passing plain char to isdigit is undefined if the value is negative. >>> >>>libstdc++-v3/ChangeLog: >>> >>> * include/std/charconv (__from_chars_alnum): Pass unsigned >>> char to std::isdigit. >>> >>>Tested powerpc64le-linux. Committed to trunk. >>> >> unsigned char __c = *__first; >>- if (std::isdigit(__c)) >>+ if (std::isdigit(static_cast<unsigned char>(__c))) >> >>I am very curious to know what this static_cast<unsigned char> does >>on __c which is already unsigned char ? If it does I'll just start >>to hate C++ :-) >> >>Maybe you wanted to put it on the previous *__first ? > >Ugh, yes, but it's not even needed there because the implicit >conversion is fine. > >We do need to fix the isspace calls in src/c++11/debug.cc but this one >was already correct. Thanks! I've reverted that useless change, thanks for noticing it.
diff --git a/libstdc++-v3/include/std/charconv b/libstdc++-v3/include/std/charconv index 193702e677a..571be075a6b 100644 --- a/libstdc++-v3/include/std/charconv +++ b/libstdc++-v3/include/std/charconv @@ -565,7 +565,7 @@ namespace __detail while (__first != __last) { unsigned char __c = *__first; - if (std::isdigit(__c)) + if (std::isdigit(static_cast<unsigned char>(__c))) __c -= '0'; else {
Passing plain char to isdigit is undefined if the value is negative. libstdc++-v3/ChangeLog: * include/std/charconv (__from_chars_alnum): Pass unsigned char to std::isdigit. Tested powerpc64le-linux. Committed to trunk. commit d0d6ca019717305df0ef41e3fe1da48f7f561fac Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed May 5 11:19:55 2021 libstdc++: Use unsigned char argument to std::isdigit Passing plain char to isdigit is undefined if the value is negative. libstdc++-v3/ChangeLog: * include/std/charconv (__from_chars_alnum): Pass unsigned char to std::isdigit.