[committed] libstdc++: Use unsigned char argument to std::isdigit

[committed] libstdc++: Use unsigned char argument to std::isdigit

[Jonathan Wakely]

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

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.


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

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.

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
 	    {

Re: [committed] libstdc++: Use unsigned char argument to std::isdigit

[François Dumont]

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 ?

Re: [committed] libstdc++: Use unsigned char argument to std::isdigit

[Jonathan Wakely]

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!

Re: [committed] libstdc++: Use unsigned char argument to std::isdigit

[Jonathan Wakely]

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.