summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEdward Welbourne <edward.welbourne@qt.io>2020-07-08 15:20:16 +0200
committerEdward Welbourne <edward.welbourne@qt.io>2020-07-14 14:52:08 +0200
commit3f8eae848e5cf80b45d226ad3ca07664df0f55c6 (patch)
tree08226f5301566531ff35fe3bb19eb6c0e8d9e27f
parent3809643093195e8750b13216ec6bf6e24d464814 (diff)
Fix floating-point 'g'-format's choice between 'e' and 'f' forms
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>
-rw-r--r--src/corelib/text/qlocale.cpp122
-rw-r--r--src/corelib/text/qlocale_tools.cpp2
-rw-r--r--tests/auto/corelib/text/qlocale/tst_qlocale.cpp48
3 files changed, 122 insertions, 50 deletions
diff --git a/src/corelib/text/qlocale.cpp b/src/corelib/text/qlocale.cpp
index ec5015a077..79291153d5 100644
--- a/src/corelib/text/qlocale.cpp
+++ b/src/corelib/text/qlocale.cpp
@@ -3348,9 +3348,6 @@ QString QLocaleData::doubleToString(const QString &zero, const QString &plus, co
if (width < 0)
width = 0;
- bool negative = false;
- QString num_str;
-
int decpt;
int bufSize = 1;
if (precision == QLocale::FloatingPointShortest)
@@ -3362,12 +3359,13 @@ QString QLocaleData::doubleToString(const QString &zero, const QString &plus, co
QVarLengthArray<char> buf(bufSize);
int length;
-
+ bool negative = false;
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) {
- num_str = QString::fromLatin1(buf.data(), length);
- } else { // Handle normal numbers
+ numStr = QString::fromLatin1(buf.data(), length);
+ } else { // Handle finite values
QString digits = QString::fromLatin1(buf.data(), length);
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;
}
- bool always_show_decpt = (flags & ForcePoint);
+ const bool mustMarkDecimal = flags & ForcePoint;
+ const bool groupDigits = flags & ThousandsGroup;
switch (form) {
- case DFExponent: {
- num_str = 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,
+ case DFExponent:
+ numStr = exponentForm(zero, decimal, exponential, group, plus, minus,
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;
- }
case DFSignificantDigits: {
PrecisionMode mode = (flags & AddTrailingZeroes) ?
PMSignificantDigits : PMChopTrailingZeros;
- const auto digitWidth = zero.size();
- int cutoff = precision < 0 ? 6 : precision;
- // Find out which representation is shorter
- if (precision == QLocale::FloatingPointShortest && decpt > 0) {
- cutoff = digits.length() / digitWidth + 4; // 'e', '+'/'-', one digit exponent
- if (decpt <= 10)
- ++cutoff;
- else
- cutoff += decpt > 100 ? 2 : 1;
- if (!always_show_decpt && digits.length() / digitWidth > decpt)
- ++cutoff; // decpt shown in exponent form, but not in decimal form
+ const int minExponentDigits = flags & ZeroPadExponent ? 2 : 1;
+
+ /* POSIX specifies sprintf() to follow fprintf(), whose 'g/G'
+ format says; with P = 6 if precision unspecified else 1 if
+ precision is 0 else precision; when 'e/E' would have exponent
+ X, use:
+ * 'f/F' if P > X >= -4, with precision P-1-X
+ * 'e/E' otherwise, with precision P-1
+ Helpfully, we already have mapped precision < 0 to 6 - except
+ for F.P.Shortest mode, which is its own story - and those of
+ 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))
- num_str = exponentForm(zero, decimal, exponential, group, plus, minus,
- digits, decpt, precision, mode,
- always_show_decpt, flags & ZeroPadExponent);
- else
- num_str = decimalForm(zero, decimal, group,
- digits, decpt, precision, mode,
- always_show_decpt, flags & ThousandsGroup);
+ numStr = useDecimal
+ ? decimalForm(zero, decimal, group,
+ digits, decpt, precision, mode,
+ mustMarkDecimal, groupDigits)
+ : exponentForm(zero, decimal, exponential, group, plus, minus,
+ digits, decpt, precision, mode,
+ mustMarkDecimal, flags & ZeroPadExponent);
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 special numbers
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
if (negative
|| flags & QLocaleData::AlwaysShowSign
@@ -3448,22 +3484,22 @@ QString QLocaleData::doubleToString(const QString &zero, const QString &plus, co
--num_pad_chars;
for (int i = 0; i < num_pad_chars; ++i)
- num_str.prepend(zero);
+ numStr.prepend(zero);
}
}
// add sign
if (negative)
- num_str.prepend(minus);
+ numStr.prepend(minus);
else if (flags & QLocaleData::AlwaysShowSign)
- num_str.prepend(plus);
+ numStr.prepend(plus);
else if (flags & QLocaleData::BlankBeforePositive)
- num_str.prepend(QLatin1Char(' '));
+ numStr.prepend(QLatin1Char(' '));
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,
diff --git a/src/corelib/text/qlocale_tools.cpp b/src/corelib/text/qlocale_tools.cpp
index 874f42c8e5..e6ecadbe4e 100644
--- a/src/corelib/text/qlocale_tools.cpp
+++ b/src/corelib/text/qlocale_tools.cpp
@@ -496,6 +496,7 @@ QString &decimalForm(const QString &zero, const QString &decimal, const QString
digits.append(zero);
break;
case PMChopTrailingZeros:
+ Q_ASSERT(digits.length() / digitWidth <= qMax(decpt, 1) || !digits.endsWith(zero));
break;
}
@@ -536,6 +537,7 @@ QString &exponentForm(const QString &zero, const QString &decimal, const QString
digits.append(zero);
break;
case PMChopTrailingZeros:
+ Q_ASSERT(digits.length() / digitWidth <= 1 || !digits.endsWith(zero));
break;
}
diff --git a/tests/auto/corelib/text/qlocale/tst_qlocale.cpp b/tests/auto/corelib/text/qlocale/tst_qlocale.cpp
index 073d0548fa..551d63c269 100644
--- a/tests/auto/corelib/text/qlocale/tst_qlocale.cpp
+++ b/tests/auto/corelib/text/qlocale/tst_qlocale.cpp
@@ -1037,8 +1037,8 @@ void tst_QLocale::stringToFloat()
void tst_QLocale::doubleToString_data()
{
- QTest::addColumn<QString>("locale_name");
- QTest::addColumn<QString>("num_str");
+ QTest::addColumn<QString>("localeName");
+ QTest::addColumn<QString>("numStr");
QTest::addColumn<double>("num");
QTest::addColumn<char>("mode");
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 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;
+ // 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 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 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;
+ // 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 -") << QString("C") << QString("12456789012") << 12456789012.0 << 'f' << shortest;
@@ -1131,8 +1165,8 @@ void tst_QLocale::doubleToString_data()
void tst_QLocale::doubleToString()
{
- QFETCH(QString, locale_name);
- QFETCH(QString, num_str);
+ QFETCH(QString, localeName);
+ QFETCH(QString, numStr);
QFETCH(double, num);
QFETCH(char, mode);
QFETCH(int, precision);
@@ -1142,11 +1176,11 @@ void tst_QLocale::doubleToString()
QSKIP("'Shortest' double conversion is not that short without libdouble-conversion");
#endif
- const QLocale locale(locale_name);
- QCOMPARE(locale.toString(num, mode, precision), num_str);
+ const QLocale locale(localeName);
+ QCOMPARE(locale.toString(num, mode, precision), numStr);
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()