Enhance precision of the FP conversion to strings in QVariant

This now guarantees that doing a round-trip from an FP number to string
and back to number results in the same number. This change is required
because DBL_DIG and FLT_DIG don't have the meaning that we were
expecting them to, here: they mean the minimum number of digits of
precision in decimal (i.e., changing the last decimal will always cause
the FP number to change). We need the maximum number: there is one
change in the last decimal place that causes the FP number to change.

IEEE 754 single-precision has 24 binary digits and double precision has
53 binary digits in their mantissa. To convert that to decimal, multiply
by the number of decimal digits a binary digit represents (log2(10) =
0.3), then add one for the rounding and one more digit for the actual
precision we want. That is, for floats we now ask for 9 digits and for
double, 17 decimal digits.

Task-number: QTBUG-42574
Change-Id: Ic78beb60a218f75322f832d33d63fd84e7a65b65
Reviewed-by: Lars Knoll <lars.knoll@digia.com>
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@digia.com>

2 files changed, 86 insertions, 46 deletions

diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp
index 2ac1bb11fb..73a951c39b 100644
--- a/src/corelib/kernel/qvariant.cpp
+++ b/src/corelib/kernel/qvariant.cpp
@@ -69,13 +69,18 @@
 
 QT_BEGIN_NAMESPACE
 
-#ifndef DBL_DIG
-#  define DBL_DIG 10
+#ifndef DBL_MANT_DIG
+#  define DBL_MANT_DIG  53
 #endif
-#ifndef FLT_DIG
-#  define FLT_DIG 6
+#ifndef FLT_MANT_DIG
+#  define FLT_MANT_DIG  24
 #endif
 
+const int log10_2_10000 = 30103;    // log10(2) * 100000
+// same as C++11 std::numeric_limits<T>::max_digits10
+const int max_digits10_double = (DBL_MANT_DIG * log10_2_10000) / 100000 + 2;
+const int max_digits10_float = (FLT_MANT_DIG * log10_2_10000) / 100000 + 2;
+
 namespace {
 class HandlersManager
 {
@@ -355,10 +360,10 @@ static bool convert(const QVariant::Private *d, int t, void *result, bool *ok)
             *str = QString::number(qMetaTypeUNumber(d));
             break;
         case QMetaType::Float:
-            *str = QString::number(d->data.f, 'g', FLT_DIG);
+            *str = QString::number(d->data.f, 'g', max_digits10_float);
             break;
         case QVariant::Double:
-            *str = QString::number(d->data.d, 'g', DBL_DIG);
+            *str = QString::number(d->data.d, 'g', max_digits10_double);
             break;
 #if !defined(QT_NO_DATESTRING)
         case QVariant::Date:
@@ -535,10 +540,10 @@ static bool convert(const QVariant::Private *d, int t, void *result, bool *ok)
             *ba = v_cast<QString>(d)->toUtf8();
             break;
         case QVariant::Double:
-            *ba = QByteArray::number(d->data.d, 'g', DBL_DIG);
+            *ba = QByteArray::number(d->data.d, 'g', max_digits10_double);
             break;
         case QMetaType::Float:
-            *ba = QByteArray::number(d->data.f, 'g', FLT_DIG);
+            *ba = QByteArray::number(d->data.f, 'g', max_digits10_float);
             break;
         case QMetaType::Char:
         case QMetaType::SChar:
diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
index 4264c96745..10c7e425f0 100644
--- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
+++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
@@ -46,6 +46,7 @@
 
 
 #include <limits.h>
+#include <float.h>
 
 #include <QLinkedList>
 #include <QRegularExpression>
@@ -213,7 +214,10 @@ private slots:
     void toIntFromQString() const;
     void toIntFromDouble() const;
     void setValue();
+    void fpStringRoundtrip_data() const;
+    void fpStringRoundtrip() const;
 
+    void numericalConvert_data();
     void numericalConvert();
     void moreCustomTypes();
     void movabilityTest();
@@ -985,7 +989,7 @@ void tst_QVariant::toByteArray_data()
     QTest::newRow( "int" ) << QVariant( -123 ) << QByteArray( "-123" );
     QTest::newRow( "uint" ) << QVariant( (uint)123 ) << QByteArray( "123" );
     QTest::newRow( "double" ) << QVariant( 123.456 ) << QByteArray( "123.456" );
-    QTest::newRow( "float" ) << QVariant( 123.456f ) << QByteArray( "123.456" );
+    QTest::newRow( "float" ) << QVariant( 123.456f ) << QByteArray( "123.456001" );
     QTest::newRow( "longlong" ) << QVariant( (qlonglong)34 ) << QByteArray( "34" );
     QTest::newRow( "ulonglong" ) << QVariant( (qulonglong)34 ) << QByteArray( "34" );
 }
@@ -1011,7 +1015,7 @@ void tst_QVariant::toString_data()
     QTest::newRow( "int" ) << QVariant( -123 ) << QString( "-123" );
     QTest::newRow( "uint" ) << QVariant( (uint)123 ) << QString( "123" );
     QTest::newRow( "double" ) << QVariant( 123.456 ) << QString( "123.456" );
-    QTest::newRow( "float" ) << QVariant( 123.456f ) << QString( "123.456" );
+    QTest::newRow( "float" ) << QVariant( 123.456f ) << QString( "123.456001" );
     QTest::newRow( "bool" ) << QVariant( true ) << QString( "true" );
     QTest::newRow( "qdate" ) << QVariant( QDate( 2002, 1, 1 ) ) << QString( "2002-01-01" );
     QTest::newRow( "qtime" ) << QVariant( QTime( 12, 34, 56 ) ) << QString( "12:34:56" );
@@ -1367,12 +1371,12 @@ void tst_QVariant::operator_eq_eq_data()
     QVariant mUIntQString(QString("42"));
 
     QVariant mDouble(42.11);
-    QVariant mDoubleString(QByteArray("42.11"));
-    QVariant mDoubleQString(QString("42.11"));
+    QVariant mDoubleString(QByteArray("42.109999999999999"));
+    QVariant mDoubleQString(QString("42.109999999999999"));
 
     QVariant mFloat(42.11f);
-    QVariant mFloatString(QByteArray("42.11"));
-    QVariant mFloatQString(QString("42.11"));
+    QVariant mFloatString(QByteArray("42.1100006"));
+    QVariant mFloatQString(QString("42.1100006"));
 
     QVariant mLongLong((qlonglong)-42);
     QVariant mLongLongString(QByteArray("-42"));
@@ -2945,41 +2949,72 @@ void tst_QVariant::setValue()
     QVERIFY( v2.isDetached() );
 }
 
+void tst_QVariant::fpStringRoundtrip_data() const
+{
+    QTest::addColumn<QVariant>("number");
+
+    QTest::newRow("float") << QVariant(1 + FLT_EPSILON);
+    QTest::newRow("double") << QVariant(1 + DBL_EPSILON);
+}
+
+void tst_QVariant::fpStringRoundtrip() const
+{
+    QFETCH(QVariant, number);
+
+    QVariant converted = number;
+    QVERIFY(converted.convert(QVariant::String));
+    QVERIFY(converted.convert(number.type()));
+    QCOMPARE(converted, number);
+
+    converted = number;
+    QVERIFY(converted.convert(QVariant::ByteArray));
+    QVERIFY(converted.convert(number.type()));
+    QCOMPARE(converted, number);
+}
+
+void tst_QVariant::numericalConvert_data()
+{
+    QTest::addColumn<QVariant>("v");
+    QTest::addColumn<bool>("isInteger");
+    QTest::newRow("float") << QVariant(float(5.3)) << false;
+    QTest::newRow("double") << QVariant(double(5.3)) << false;
+    QTest::newRow("qreal") << QVariant(qreal(5.3)) << false;
+    QTest::newRow("int") << QVariant(int(5)) << true;
+    QTest::newRow("uint") << QVariant(uint(5)) << true;
+    QTest::newRow("short") << QVariant(short(5)) << true;
+    QTest::newRow("longlong") << QVariant(quint64(5)) << true;
+    QTest::newRow("long") << QVariant::fromValue(long(5)) << true;
+    QTest::newRow("stringint") << QVariant(QString::fromLatin1("5")) << true;
+    QTest::newRow("string") << QVariant(QString::fromLatin1("5.30000019")) << false;
+}
+
 void tst_QVariant::numericalConvert()
 {
 #if defined(Q_OS_LINUX) && defined(Q_CC_GNU) && !defined(__x86_64__)
     QSKIP("Known to fail due to a GCC bug on at least Ubuntu 10.04 32-bit - check QTBUG-8959");
 #endif
-    QVariant vfloat(float(5.3));
-    QVariant vdouble(double(5.3));
-    QVariant vreal(qreal(5.3));
-    QVariant vint(int(5));
-    QVariant vuint(uint(5));
-    QVariant vshort(short(5));
-    QVariant vlonglong(quint64(5));
-    QVariant vlong = QVariant::fromValue(long(5));
-    QVariant vstringint(QString::fromLatin1("5"));
-    QVariant vstring(QString::fromLatin1("5.3"));
-
-    QVector<QVariant *> vect;
-    vect << &vfloat << &vdouble << &vreal << &vint << &vuint << &vshort<< &vlonglong << &vlong << &vstringint << &vstring;
-
-    for(int i = 0; i < vect.size(); i++) {
-        double num = 5.3;
-        if (i >= 3 && i <= 8)
-            num = 5;
-        QVariant *v = vect.at(i);
-        QCOMPARE(v->toFloat() , float(num));
-        QCOMPARE(float(v->toReal()) , float(num));
-        QCOMPARE(float(v->toDouble()) , float(num));
-        if (i != 9) {
-            QCOMPARE(v->toInt() , int(num));
-            QCOMPARE(v->toUInt() , uint(num));
-            QCOMPARE(v->toULongLong() , quint64(num));
-            QCOMPARE(v->value<ulong>() , ulong(num));
-            QCOMPARE(v->value<ushort>() , ushort(num));
-        }
-        QCOMPARE(v->toString() , QString::number(num));
+    QFETCH(QVariant, v);
+    QFETCH(bool, isInteger);
+    double num = isInteger ? 5 : 5.3;
+
+    QCOMPARE(v.toFloat() , float(num));
+    QCOMPARE(float(v.toReal()) , float(num));
+    QCOMPARE(float(v.toDouble()) , float(num));
+    if (isInteger) {
+        QCOMPARE(v.toInt() , int(num));
+        QCOMPARE(v.toUInt() , uint(num));
+        QCOMPARE(v.toULongLong() , quint64(num));
+        QCOMPARE(v.value<ulong>() , ulong(num));
+        QCOMPARE(v.value<ushort>() , ushort(num));
+    }
+    switch (v.userType())
+    {
+    case QVariant::Double:
+        QCOMPARE(v.toString() , QString::number(num, 'g', DBL_MANT_DIG * log10(2.) + 2));
+        break;
+    case QMetaType::Float:
+        QCOMPARE(v.toString() , QString::number(float(num), 'g', FLT_MANT_DIG * log10(2.) + 2));
+        break;
     }
 }
 
@@ -3214,8 +3249,8 @@ void tst_QVariant::moreCustomTypes()
     QCOMPARE(MyNotMovable::count, 0);
 
     {
-        PLAY_WITH_VARIANT(12.12, false, "12.12", 12.12, true);
-        PLAY_WITH_VARIANT(12.12f, false, "12.12", 12.12f, true);
+        PLAY_WITH_VARIANT(12.12, false, "12.119999999999999", 12.12, true);
+        PLAY_WITH_VARIANT(12.12f, false, "12.1199999", 12.12f, true);
         PLAY_WITH_VARIANT('a', false, "a", 'a', true);
         PLAY_WITH_VARIANT((unsigned char)('a'), false, "a", 'a', true);
         PLAY_WITH_VARIANT( quint8(12), false, "\xc", 12, true);