AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix floating-point 'g'-format's choice between 'e' and 'f' forms

Browse Source 
During review of a refactor (coming shortly), Thiago wondered what the
magic numbers were. On closer examination, I concluded that they were
wrong and wrote some tests to prove it. This commit adds those tests;
replaces the misguided old code with something that passes them; and
documents the reasons for the various parts of its decisions.

In the process, tidy up QLocaleData::doubleToString() somewhat and
rename some of its variables to conform to Qt coding style.

Change-Id: Ibee43659b1bdb0707639cdb444cfe941c31d409f
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Edward Welbourne 2020-07-08 15:20:16 +02:00
parent 3809643093
commit 3f8eae848e
3 changed files with 122 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

122
src/corelib/text/qlocale.cpp
View File

@ -3348,9 +3348,6 @@ QString QLocaleData::doubleToString(const QString &zero, const QString &plus, co
if (width < 0) if (width < 0)
width = 0; width = 0;
bool negative = false;
QString num_str;
int decpt; int decpt;
int bufSize = 1; int bufSize = 1;
if (precision == QLocale::FloatingPointShortest) if (precision == QLocale::FloatingPointShortest)
@ -3362,12 +3359,13 @@ QString QLocaleData::doubleToString(const QString &zero, const QString &plus, co
QVarLengthArray<char> buf(bufSize); QVarLengthArray<char> buf(bufSize);
int length; int length;
bool negative = false;
qt_doubleToAscii(d, form, precision, buf.data(), bufSize, negative, length, decpt); qt_doubleToAscii(d, form, precision, buf.data(), bufSize, negative, length, decpt);
QString numStr;
if (qstrncmp(buf.data(), "inf", 3) == 0 || qstrncmp(buf.data(), "nan", 3) == 0) { if (qstrncmp(buf.data(), "inf", 3) == 0 || qstrncmp(buf.data(), "nan", 3) == 0) {
num_str = QString::fromLatin1(buf.data(), length); numStr = QString::fromLatin1(buf.data(), length);
} else { // Handle normal numbers } else { // Handle finite values
QString digits = QString::fromLatin1(buf.data(), length); QString digits = QString::fromLatin1(buf.data(), length);
if (zero == u"0") { if (zero == u"0") {
@ -3391,45 +3389,83 @@ QString QLocaleData::doubleToString(const QString &zero, const QString &plus, co
reinterpret_cast<ushort *>(digits.data())[i] += z; reinterpret_cast<ushort *>(digits.data())[i] += z;
} }
bool always_show_decpt = (flags & ForcePoint); const bool mustMarkDecimal = flags & ForcePoint;
const bool groupDigits = flags & ThousandsGroup;
switch (form) { switch (form) {
case DFExponent: { case DFExponent:
num_str = exponentForm(zero, decimal, exponential, group, plus, minus, numStr = exponentForm(zero, decimal, exponential, group, plus, minus,
digits, decpt, precision, PMDecimalDigits,
always_show_decpt, flags & ZeroPadExponent);
break;
}
case DFDecimal: {
num_str = decimalForm(zero, decimal, group,
digits, decpt, precision, PMDecimalDigits, digits, decpt, precision, PMDecimalDigits,
always_show_decpt, flags & ThousandsGroup); mustMarkDecimal, flags & ZeroPadExponent);
break;
case DFDecimal:
numStr = decimalForm(zero, decimal, group,
digits, decpt, precision, PMDecimalDigits,
mustMarkDecimal, groupDigits);
break; break;
}
case DFSignificantDigits: { case DFSignificantDigits: {
PrecisionMode mode = (flags & AddTrailingZeroes) ? PrecisionMode mode = (flags & AddTrailingZeroes) ?
PMSignificantDigits : PMChopTrailingZeros; PMSignificantDigits : PMChopTrailingZeros;
const auto digitWidth = zero.size(); const int minExponentDigits = flags & ZeroPadExponent ? 2 : 1;
int cutoff = precision < 0 ? 6 : precision;
// Find out which representation is shorter /* POSIX specifies sprintf() to follow fprintf(), whose 'g/G'
if (precision == QLocale::FloatingPointShortest && decpt > 0) { format says; with P = 6 if precision unspecified else 1 if
cutoff = digits.length() / digitWidth + 4; // 'e', '+'/'-', one digit exponent precision is 0 else precision; when 'e/E' would have exponent
if (decpt <= 10) X, use:
++cutoff; * 'f/F' if P > X >= -4, with precision P-1-X
else * 'e/E' otherwise, with precision P-1
cutoff += decpt > 100 ? 2 : 1; Helpfully, we already have mapped precision < 0 to 6 - except
if (!always_show_decpt && digits.length() / digitWidth > decpt) for F.P.Shortest mode, which is its own story - and those of
++cutoff; // decpt shown in exponent form, but not in decimal form our callers with unspecified precision either used 6 or -1
for it.
*/
bool useDecimal;
if (precision == QLocale::FloatingPointShortest) {
// Find out which representation is shorter.
// Set bias to everything added to exponent form but not
// decimal, minus the converse.
// Exponent adds separator, sign and digits:
int bias = 2 + minExponentDigits;
// Decimal form may get grouping separators inserted:
if (groupDigits && decpt > 3)
bias -= (decpt - 1) / 3;
// X = decpt - 1 needs two digits if decpt > 10:
if (decpt > 10 && minExponentDigits == 1)
++bias;
// Assume digitCount < 95, so we can ignore the 3-digit
// exponent case (we'll set useDecimal false anyway).
const int digitCount = digits.length() / zero.size();
if (!mustMarkDecimal) {
// Decimal separator is skipped if at end; adjust if
// that happens for only one form:
if (digitCount <= decpt && digitCount > 1)
++bias; // decimal but not exponent
else if (digitCount == 1 && decpt <= 0)
--bias; // exponent but not decimal
}
// When 0 < decpt <= digitCount, the forms have equal digit
// counts, plus things bias has taken into account;
// otherwise decimal form's digit count is right-padded with
// zeros to decpt, when decpt is positive, otherwise it's
// left-padded with 1 - decpt zeros.
useDecimal = (decpt <= 0 ? 1 - decpt <= bias
: decpt <= digitCount ? 0 <= bias
: decpt <= digitCount + bias);
} else {
// X == decpt - 1, POSIX's P; -4 <= X < P iff -4 < decpt <= P
Q_ASSERT(precision >= 0);
useDecimal = decpt > -4 && decpt <= (precision ? precision : 1);
} }
if (decpt != digits.length() / digitWidth && (decpt <= -4 || decpt > cutoff)) numStr = useDecimal
num_str = exponentForm(zero, decimal, exponential, group, plus, minus, ? decimalForm(zero, decimal, group,
digits, decpt, precision, mode, digits, decpt, precision, mode,
always_show_decpt, flags & ZeroPadExponent); mustMarkDecimal, groupDigits)
else : exponentForm(zero, decimal, exponential, group, plus, minus,
num_str = decimalForm(zero, decimal, group, digits, decpt, precision, mode,
digits, decpt, precision, mode, mustMarkDecimal, flags & ZeroPadExponent);
always_show_decpt, flags & ThousandsGroup);
break; break;
} }
} }
@ -3440,7 +3476,7 @@ QString QLocaleData::doubleToString(const QString &zero, const QString &plus, co
// pad with zeros. LeftAdjusted overrides this flag. Also, we don't // pad with zeros. LeftAdjusted overrides this flag. Also, we don't
// pad special numbers // pad special numbers
if (flags & QLocaleData::ZeroPadded && !(flags & QLocaleData::LeftAdjusted)) { if (flags & QLocaleData::ZeroPadded && !(flags & QLocaleData::LeftAdjusted)) {
int num_pad_chars = width - num_str.length() / zero.length(); int num_pad_chars = width - numStr.length() / zero.length();
// leave space for the sign // leave space for the sign
if (negative if (negative
|| flags & QLocaleData::AlwaysShowSign || flags & QLocaleData::AlwaysShowSign
@ -3448,22 +3484,22 @@ QString QLocaleData::doubleToString(const QString &zero, const QString &plus, co
--num_pad_chars; --num_pad_chars;
for (int i = 0; i < num_pad_chars; ++i) for (int i = 0; i < num_pad_chars; ++i)
num_str.prepend(zero); numStr.prepend(zero);
} }
} }
// add sign // add sign
if (negative) if (negative)
num_str.prepend(minus); numStr.prepend(minus);
else if (flags & QLocaleData::AlwaysShowSign) else if (flags & QLocaleData::AlwaysShowSign)
num_str.prepend(plus); numStr.prepend(plus);
else if (flags & QLocaleData::BlankBeforePositive) else if (flags & QLocaleData::BlankBeforePositive)
num_str.prepend(QLatin1Char(' ')); numStr.prepend(QLatin1Char(' '));
if (flags & QLocaleData::CapitalEorX) if (flags & QLocaleData::CapitalEorX)
num_str = std::move(num_str).toUpper(); numStr = std::move(numStr).toUpper();
return num_str; return numStr;
} }
QString QLocaleData::longLongToString(qlonglong l, int precision, QString QLocaleData::longLongToString(qlonglong l, int precision,

2
src/corelib/text/qlocale_tools.cpp
View File

@ -496,6 +496,7 @@ QString &decimalForm(const QString &zero, const QString &decimal, const QString
digits.append(zero); digits.append(zero);
break; break;
case PMChopTrailingZeros: case PMChopTrailingZeros:
Q_ASSERT(digits.length() / digitWidth <= qMax(decpt, 1) || !digits.endsWith(zero));
break; break;
} }
@ -536,6 +537,7 @@ QString &exponentForm(const QString &zero, const QString &decimal, const QString
digits.append(zero); digits.append(zero);
break; break;
case PMChopTrailingZeros: case PMChopTrailingZeros:
Q_ASSERT(digits.length() / digitWidth <= 1 || !digits.endsWith(zero));
break; break;
} }

48
tests/auto/corelib/text/qlocale/tst_qlocale.cpp
View File

@ -1037,8 +1037,8 @@ void tst_QLocale::stringToFloat()
void tst_QLocale::doubleToString_data() void tst_QLocale::doubleToString_data()
{ {
QTest::addColumn<QString>("locale_name"); QTest::addColumn<QString>("localeName");
QTest::addColumn<QString>("num_str"); QTest::addColumn<QString>("numStr");
QTest::addColumn<double>("num"); QTest::addColumn<double>("num");
QTest::addColumn<char>("mode"); QTest::addColumn<char>("mode");
QTest::addColumn<int>("precision"); QTest::addColumn<int>("precision");
@ -1086,6 +1086,23 @@ void tst_QLocale::doubleToString_data()
QTest::newRow("de_DE 0,035003945 e -") << QString("de_DE") << QString("3,5003945E-02") << 0.035003945 << 'e' << shortest; QTest::newRow("de_DE 0,035003945 e -") << QString("de_DE") << QString("3,5003945E-02") << 0.035003945 << 'e' << shortest;
QTest::newRow("de_DE 0,035003945 g 8") << QString("de_DE") << QString("0,035003945") << 0.035003945 << 'g' << 8; QTest::newRow("de_DE 0,035003945 g 8") << QString("de_DE") << QString("0,035003945") << 0.035003945 << 'g' << 8;
QTest::newRow("de_DE 0,035003945 g -") << QString("de_DE") << QString("0,035003945") << 0.035003945 << 'g' << shortest; QTest::newRow("de_DE 0,035003945 g -") << QString("de_DE") << QString("0,035003945") << 0.035003945 << 'g' << shortest;
// Check 'f/F' iff (adjusted) precision > exponent >= -4:
QTest::newRow("de_DE 12345 g 4") << QString("de_DE") << QString("1,235E+04") << 12345. << 'g' << 4;
QTest::newRow("de_DE 1e7 g 8") << QString("de_DE") << QString("10.000.000") << 1e7 << 'g' << 8;
QTest::newRow("de_DE 1e8 g 8") << QString("de_DE") << QString("1E+08") << 1e8 << 'g' << 8;
QTest::newRow("de_DE 10.0 g 1") << QString("de_DE") << QString("1E+01") << 10.0 << 'g' << 1;
QTest::newRow("de_DE 10.0 g 0") << QString("de_DE") << QString("1E+01") << 10.0 << 'g' << 0;
QTest::newRow("de_DE 1.0 g 0") << QString("de_DE") << QString("1") << 1.0 << 'g' << 0;
QTest::newRow("de_DE 0.0001 g 0") << QString("de_DE") << QString("0,0001") << 0.0001 << 'g' << 0;
QTest::newRow("de_DE 0.00001 g 0") << QString("de_DE") << QString("1E-05") << 0.00001 << 'g' << 0;
// Check transition to exponent form:
QTest::newRow("de_DE 1245678900 g -") << QString("de_DE") << QString("1.245.678.900") << 12456789e2 << 'g' << shortest;
QTest::newRow("de_DE 12456789100 g -") << QString("de_DE") << QString("12.456.789.100") << 124567891e2 << 'g' << shortest;
QTest::newRow("de_DE 12456789000 g -") << QString("de_DE") << QString("1,2456789E+10") << 12456789e3 << 'g' << shortest;
QTest::newRow("de_DE 120000 g -") << QString("de_DE") << QString("120.000") << 12e4 << 'g' << shortest;
QTest::newRow("de_DE 1200000 g -") << QString("de_DE") << QString("1,2E+06") << 12e5 << 'g' << shortest;
QTest::newRow("de_DE 1000 g -") << QString("de_DE") << QString("1.000") << 1e3 << 'g' << shortest;
QTest::newRow("de_DE 10000 g -") << QString("de_DE") << QString("1E+04") << 1e4 << 'g' << shortest;
QTest::newRow("C 0.000003945 f 12") << QString("C") << QString("0.000003945000") << 0.000003945 << 'f' << 12; QTest::newRow("C 0.000003945 f 12") << QString("C") << QString("0.000003945000") << 0.000003945 << 'f' << 12;
QTest::newRow("C 0.000003945 f 6") << QString("C") << QString("0.000004") << 0.000003945 << 'f' << 6; QTest::newRow("C 0.000003945 f 6") << QString("C") << QString("0.000004") << 0.000003945 << 'f' << 6;
@ -1113,6 +1130,23 @@ void tst_QLocale::doubleToString_data()
QTest::newRow("C 12456789012 e 7") << QString("C") << QString("1.2456789e+10") << 12456789012.0 << 'e' << 7; QTest::newRow("C 12456789012 e 7") << QString("C") << QString("1.2456789e+10") << 12456789012.0 << 'e' << 7;
QTest::newRow("C 12456789012 g 14") << QString("C") << QString("12456789012") << 12456789012.0 << 'g' << 14; QTest::newRow("C 12456789012 g 14") << QString("C") << QString("12456789012") << 12456789012.0 << 'g' << 14;
QTest::newRow("C 12456789012 g 8") << QString("C") << QString("1.2456789e+10") << 12456789012.0 << 'g' << 8; QTest::newRow("C 12456789012 g 8") << QString("C") << QString("1.2456789e+10") << 12456789012.0 << 'g' << 8;
// Check 'f/F' iff (adjusted) precision > exponent >= -4:
QTest::newRow("C 12345 g 4") << QString("C") << QString("1.235e+04") << 12345. << 'g' << 4;
QTest::newRow("C 1e7 g 8") << QString("C") << QString("10000000") << 1e7 << 'g' << 8;
QTest::newRow("C 1e8 g 8") << QString("C") << QString("1e+08") << 1e8 << 'g' << 8;
QTest::newRow("C 10.0 g 1") << QString("C") << QString("1e+01") << 10.0 << 'g' << 1;
QTest::newRow("C 10.0 g 0") << QString("C") << QString("1e+01") << 10.0 << 'g' << 0;
QTest::newRow("C 1.0 g 0") << QString("C") << QString("1") << 1.0 << 'g' << 0;
QTest::newRow("C 0.0001 g 0") << QString("C") << QString("0.0001") << 0.0001 << 'g' << 0;
QTest::newRow("C 0.00001 g 0") << QString("C") << QString("1e-05") << 0.00001 << 'g' << 0;
// Check transition to exponent form:
QTest::newRow("C 1245678900000 g -") << QString("C") << QString("1245678900000") << 1245678900000.0 << 'g' << shortest;
QTest::newRow("C 12456789100000 g -") << QString("C") << QString("12456789100000") << 12456789100000.0 << 'g' << shortest;
QTest::newRow("C 12456789000000 g -") << QString("C") << QString("1.2456789e+13") << 12456789000000.0 << 'g' << shortest;
QTest::newRow("C 1200000 g -") << QString("C") << QString("1200000") << 12e5 << 'g' << shortest;
QTest::newRow("C 12000000 g -") << QString("C") << QString("1.2e+07") << 12e6 << 'g' << shortest;
QTest::newRow("C 10000 g -") << QString("C") << QString("10000") << 1e4 << 'g' << shortest;
QTest::newRow("C 100000 g -") << QString("C") << QString("1e+05") << 1e5 << 'g' << shortest;
QTest::newRow("C 12456789012 f 0") << QString("C") << QString("12456789012") << 12456789012.0 << 'f' << 0; QTest::newRow("C 12456789012 f 0") << QString("C") << QString("12456789012") << 12456789012.0 << 'f' << 0;
QTest::newRow("C 12456789012 f -") << QString("C") << QString("12456789012") << 12456789012.0 << 'f' << shortest; QTest::newRow("C 12456789012 f -") << QString("C") << QString("12456789012") << 12456789012.0 << 'f' << shortest;
@ -1131,8 +1165,8 @@ void tst_QLocale::doubleToString_data()
void tst_QLocale::doubleToString() void tst_QLocale::doubleToString()
{ {
QFETCH(QString, locale_name); QFETCH(QString, localeName);
QFETCH(QString, num_str); QFETCH(QString, numStr);
QFETCH(double, num); QFETCH(double, num);
QFETCH(char, mode); QFETCH(char, mode);
QFETCH(int, precision); QFETCH(int, precision);
@ -1142,11 +1176,11 @@ void tst_QLocale::doubleToString()
QSKIP("'Shortest' double conversion is not that short without libdouble-conversion"); QSKIP("'Shortest' double conversion is not that short without libdouble-conversion");
#endif #endif
const QLocale locale(locale_name); const QLocale locale(localeName);
QCOMPARE(locale.toString(num, mode, precision), num_str); QCOMPARE(locale.toString(num, mode, precision), numStr);
TransientLocale ignoreme(LC_ALL, "de_DE"); TransientLocale ignoreme(LC_ALL, "de_DE");
QCOMPARE(locale.toString(num, mode, precision), num_str); QCOMPARE(locale.toString(num, mode, precision), numStr);
} }
void tst_QLocale::strtod_data() void tst_QLocale::strtod_data()