Priority: P4: Low
Affects Version/s: 5.6.0
QLocaleData::longLongToString() suppresses AlwaysShowSign and BlankBeforePositive for bases other than ten on grounds "these are not supported by sprintf for octal and hex"; and it suppresses handling of negative (casting to unsigned, so that -17 becomes 0xffffffffffffffef) on the same grounds.
I contend that this is entirely bogus behaviour.
The sprintf reference (which could as well be to any member of the whole printf family) can only be to the fact that the o, u, x and X formats don't handle sign; these formats are defined to take unsigned inputs. They have no analogue for signed inputs, but that just means they provide no precedent for us to follow when we are handling a signed input. Our implementation of any printf-like function should, when processing %o, %u, %x or %X, use QLocaleData::unsLongLongToString(), so not be affected by what we chose to implement in QLocaleData::longLongToString() as handling of sign-related formatting details for bases other than ten. (Indeed, QString::vasprintf() does call the right method here, as it should.)
Since QString::number(value, base=10) accepts an optional base, we should Do The Right Thing when the value is negative, handling sign gracefully, not casting via unsigned long long.
In python, where there is no difference between signed and unsigned (only between ordinary int and infinite precision long - and even this makes no difference to string formatting), the %x format specifier cheerfully handles negative integers just fine, turning '%x' % -17 into '-11' as one would expect. This is clearly correct behaviour and a far more apt precedent for the case in hand.
I propose, therefore, that QLocaleData::longLongToString() should discard its code that borks negative input for bases other than ten; and QString::number() would then handle negative numbers sanely with base.
|For Gerrit Dashboard: QTBUG-53706|
|319069,7||Change QString formatting of negative numbers in non-base-10||dev||qt/qtbase||Status: MERGED||+2||0|
|362329,10||QString/QByteArray: Fix setNum docs and add tests||dev||qt/qtbase||Status: MERGED||+2||0|
|362684,4||QByteArray: don't coerce negative to unsigned for any base||dev||qt/qtbase||Status: MERGED||+2||0|
|362921,2||QByteArray: don't coerce negative to unsigned for any base||6.2||qt/qtbase||Status: MERGED||+2||0|
|363566,3||QString/QByteArray: Fix setNum docs and add tests||6.2||qt/qtbase||Status: MERGED||+2||0|