From 3f8eae848e5cf80b45d226ad3ca07664df0f55c6 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Wed, 8 Jul 2020 15:20:16 +0200 Subject: 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 --- src/corelib/text/qlocale.cpp | 122 ++++++++++++++++++++++++++++--------------- 1 file changed, 79 insertions(+), 43 deletions(-) (limited to 'src/corelib/text/qlocale.cpp') 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 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(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, -- cgit v1.2.3