From a9923674030980706940b3ee11145c38674bb35d Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Mon, 19 Nov 2018 16:27:03 +0100 Subject: [PATCH] Change documentation of some toDouble()s to reflect reality They actually return infinity if conversion overflows, while still setting ok to false; they were documented to return 0 on failure, with no mention of this special handling of overflow. Documented reality rather than changing the behavior. Gave underflow as an example of failure other than overflow (toDouble()s do indeed fail on it). Added some tests of out-of-range values, infinities and NaNs. [ChangeLog][QtCore][toDouble] QString, QByteArray and QLocale return an infinity on overflow (since 5.7), while setting ok to false; this was at odds with their documented behavior of returning 0 on failure. The documentation now reflects the actual behavior. Fixes: QTBUG-71256 Change-Id: I8d7e80ba1f06091cf0f1480c341553381103703b Reviewed-by: Ulf Hermann --- src/corelib/tools/qbytearray.cpp | 3 +- src/corelib/tools/qlocale.cpp | 18 ++++-- src/corelib/tools/qstring.cpp | 6 +- .../corelib/tools/qlocale/tst_qlocale.cpp | 56 +++++++++++++++---- 4 files changed, 62 insertions(+), 21 deletions(-) diff --git a/src/corelib/tools/qbytearray.cpp b/src/corelib/tools/qbytearray.cpp index 424420204a1..1b7a4366c7e 100644 --- a/src/corelib/tools/qbytearray.cpp +++ b/src/corelib/tools/qbytearray.cpp @@ -3871,7 +3871,8 @@ ushort QByteArray::toUShort(bool *ok, int base) const /*! Returns the byte array converted to a \c double value. - Returns 0.0 if the conversion fails. + Returns an infinity if the conversion overflows or 0.0 if the + conversion fails for other reasons (e.g. underflow). If \a ok is not \c nullptr, failure is reported by setting *\a{ok} to \c false, and success by setting *\a{ok} to \c true. diff --git a/src/corelib/tools/qlocale.cpp b/src/corelib/tools/qlocale.cpp index 6361280bc7f..8f9fb3de2d9 100644 --- a/src/corelib/tools/qlocale.cpp +++ b/src/corelib/tools/qlocale.cpp @@ -1373,8 +1373,10 @@ float QLocale::toFloat(const QString &s, bool *ok) const } /*! - Returns the double represented by the localized string \a s, or - 0.0 if the conversion failed. + Returns the double represented by the localized string \a s. + + Returns an infinity if the conversion overflows or 0.0 if the + conversion fails for any other reason (e.g. underflow). If \a ok is not \c nullptr, failure is reported by setting *\a{ok} to \c false, and success by setting *\a{ok} to \c true. @@ -1542,8 +1544,10 @@ float QLocale::toFloat(const QStringRef &s, bool *ok) const } /*! - Returns the double represented by the localized string \a s, or - 0.0 if the conversion failed. + Returns the double represented by the localized string \a s. + + Returns an infinity if the conversion overflows or 0.0 if the + conversion fails for any other reason (e.g. underflow). If \a ok is not \c nullptr, failure is reported by setting *\a{ok} to \c false, and success by setting *\a{ok} to \c true. @@ -1711,8 +1715,10 @@ float QLocale::toFloat(QStringView s, bool *ok) const } /*! - Returns the double represented by the localized string \a s, or - 0.0 if the conversion failed. + Returns the double represented by the localized string \a s. + + Returns an infinity if the conversion overflows or 0.0 if the + conversion fails for any other reason (e.g. underflow). If \a ok is not \c nullptr, failure is reported by setting *\a{ok} to \c false, and success by setting *\a{ok} to \c true. diff --git a/src/corelib/tools/qstring.cpp b/src/corelib/tools/qstring.cpp index 0a8b2b42383..1381e8c11c6 100644 --- a/src/corelib/tools/qstring.cpp +++ b/src/corelib/tools/qstring.cpp @@ -7169,7 +7169,8 @@ ushort QString::toUShort(bool *ok, int base) const /*! Returns the string converted to a \c double value. - Returns 0.0 if the conversion fails. + Returns an infinity if the conversion overflows or 0.0 if the + conversion fails for other reasons (e.g. underflow). If \a ok is not \c nullptr, failure is reported by setting *\a{ok} to \c false, and success by setting *\a{ok} to \c true. @@ -11794,7 +11795,8 @@ ushort QStringRef::toUShort(bool *ok, int base) const /*! Returns the string converted to a \c double value. - Returns 0.0 if the conversion fails. + Returns an infinity if the conversion overflows or 0.0 if the + conversion fails for other reasons (e.g. underflow). If \a ok is not \c nullptr, failure is reported by setting *\a{ok} to \c false, and success by setting *\a{ok} to \c true. diff --git a/tests/auto/corelib/tools/qlocale/tst_qlocale.cpp b/tests/auto/corelib/tools/qlocale/tst_qlocale.cpp index fa1a64a045b..7bf6d1327ec 100644 --- a/tests/auto/corelib/tools/qlocale/tst_qlocale.cpp +++ b/tests/auto/corelib/tools/qlocale/tst_qlocale.cpp @@ -83,7 +83,7 @@ private slots: void matchingLocales(); void stringToDouble_data(); void stringToDouble(); - void stringToFloat_data() { stringToDouble_data(); } + void stringToFloat_data(); void stringToFloat(); void doubleToString_data(); void doubleToString(); @@ -155,6 +155,7 @@ private: QString m_decimal, m_thousand, m_sdate, m_ldate, m_time; QString m_sysapp; bool europeanTimeZone; + void toReal_data(); class TransientLocale { @@ -679,7 +680,7 @@ void tst_QLocale::unixLocaleName() #undef TEST_NAME } -void tst_QLocale::stringToDouble_data() +void tst_QLocale::toReal_data() { QTest::addColumn("locale_name"); QTest::addColumn("num_str"); @@ -801,6 +802,32 @@ void tst_QLocale::stringToDouble_data() QTest::newRow("de_DE 9.876543,0e--2") << QString("de_DE") << QString("9.876543,0e")+QChar(8722)+QString("2") << false << 0.0; } +void tst_QLocale::stringToDouble_data() +{ + toReal_data(); + if (std::numeric_limits::has_infinity) { + double huge = std::numeric_limits::infinity(); + QTest::newRow("C inf") << QString("C") << QString("inf") << true << huge; + QTest::newRow("C +inf") << QString("C") << QString("+inf") << true << +huge; + QTest::newRow("C -inf") << QString("C") << QString("-inf") << true << -huge; + // Overflow: + QTest::newRow("C huge") << QString("C") << QString("2e308") << false << huge; + QTest::newRow("C -huge") << QString("C") << QString("-2e308") << false << -huge; + } + if (std::numeric_limits::has_quiet_NaN) + QTest::newRow("C qnan") << QString("C") << QString("NaN") << true << std::numeric_limits::quiet_NaN(); + + // In range (but outside float's range): + QTest::newRow("C big") << QString("C") << QString("3.5e38") << true << 3.5e38; + QTest::newRow("C -big") << QString("C") << QString("-3.5e38") << true << -3.5e38; + QTest::newRow("C small") << QString("C") << QString("1e-45") << true << 1e-45; + QTest::newRow("C -small") << QString("C") << QString("-1e-45") << true << -1e-45; + + // Underflow: + QTest::newRow("C tiny") << QString("C") << QString("2e-324") << false << 0.; + QTest::newRow("C -tiny") << QString("C") << QString("-2e-324") << false << 0.; +} + void tst_QLocale::stringToDouble() { #define MY_DOUBLE_EPSILON (2.22045e-16) // 1/2^{52}; double has a 53-bit mantissa @@ -825,28 +852,33 @@ void tst_QLocale::stringToDouble() QCOMPARE(ok, good); } - if (ok) { + if (ok || std::isinf(num)) { // First use fuzzy-compare, then a more precise check: QCOMPARE(d, num); - double diff = d - num; - if (diff < 0) - diff = -diff; - QVERIFY(diff <= MY_DOUBLE_EPSILON); + if (std::isfinite(num)) { + double diff = d > num ? d - num : num - d; + QVERIFY(diff <= MY_DOUBLE_EPSILON); + } } d = locale.toDouble(num_strRef, &ok); QCOMPARE(ok, good); - if (ok) { + if (ok || std::isinf(num)) { QCOMPARE(d, num); - double diff = d - num; - if (diff < 0) - diff = -diff; - QVERIFY(diff <= MY_DOUBLE_EPSILON); + if (std::isfinite(num)) { + double diff = d > num ? d - num : num - d; + QVERIFY(diff <= MY_DOUBLE_EPSILON); + } } #undef MY_DOUBLE_EPSILON } +void tst_QLocale::stringToFloat_data() +{ + toReal_data(); +} + void tst_QLocale::stringToFloat() { #define MY_FLOAT_EPSILON (2.384e-7) // 1/2^{22}; float has a 23-bit mantissa