Revert "Reading QJsonObject property should not modify the object itself."

This reverts commit 20cf632ad5f3ffe7b0fd231724c971f4e07304eb. The commit
produced to many problems during statics destruction. For example
causing QtCreator crash (QTBUG-40987).

Change-Id: Ib52f6a449c2d84deab2de792559a6a065ca45e8d
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

src/corelib/json/qjson_p.h

@@ -61,8 +61,6 @@
 #include <qstring.h>
 #include <qendian.h>
 #include <qnumeric.h>
-#include <QtCore/qhash.h>
-#include <QtCore/qmutex.h>
 
 #include <limits.h>
 #include <limits>
@@ -795,75 +793,6 @@ private:
     Q_DISABLE_COPY(Data)
 };
 
-struct ObjectUndefinedKeys
-{
-    static void insertKey(const QJsonObject *object, int index, const QString &key)
-    {
-        QMutexLocker lock(&mutex);
-        keys()[object][index] = key;
-    }
-    static QString takeKey(const QJsonObject *object, int index)
-    {
-        QMutexLocker lock(&mutex);
-        return keys()[object].take(index);
-    }
-    static void removeKeys(const QJsonObject *object)
-    {
-        QMutexLocker lock(&mutex);
-        keys().remove(object);
-    }
-private:
-    typedef QHash<const QJsonObject*, QHash<int, QString> > KeysHash;
-    static KeysHash &keys()
-    {
-        static KeysHash keys;
-        return keys;
-    }
-   static QBasicMutex mutex;
-};
-
-} // namespace QJsonPrivate
-
-struct QJsonValueRef::UnionHelper
-{
-    static inline QJsonObject *untaggedPointer(QJsonObject *object)
-    {
-        const quintptr Mask = ~quintptr(0) << 2;
-        return reinterpret_cast<QJsonObject*>(quintptr(object) & Mask);
-    }
-    static inline QJsonObject *taggedPointer(QJsonObject *object)
-    {
-        const quintptr Mask = 1;
-        return reinterpret_cast<QJsonObject*>(quintptr(object) | Mask);
-    }
-
-    static void setValueAt(QJsonValueRef *ref, QJsonValue value)
-    {
-        using namespace QJsonPrivate;
-        QJsonObject *object = untaggedPointer(ref->o);
-        if (ref->o != object)
-            object->insert(ObjectUndefinedKeys::takeKey(object, ref->index), value);
-        else
-            object->setValueAt(ref->index, value);
-    }
-
-    static QJsonValue valueAt(const QJsonValueRef *ref)
-    {
-        QJsonObject *object = untaggedPointer(ref->o);
-        if (ref->o != object)
-            return QJsonValue::Undefined;
-        return ref->o->valueAt(ref->index);
-    }
-};
-
-/*!
-    \internal
-    Constructor that creates reference to an undefined value in \a object.
-*/
-inline QJsonValueRef::QJsonValueRef(QJsonObject *object, const QString &key)
-    : o(UnionHelper::taggedPointer(object)), is_object(true), index(qHash(key))
-{
-    QJsonPrivate::ObjectUndefinedKeys::insertKey(object, index, key);
 }
 
 QT_END_NAMESPACE

src/corelib/json/qjsonobject.cpp

@@ -161,7 +161,6 @@ QJsonObject::~QJsonObject()
 {
     if (d && !d->ref.deref())
         delete d;
-    QJsonPrivate::ObjectUndefinedKeys::removeKeys(this); // ### Qt6 move it to ~QJsonValueRef
 }
 
 /*!
@@ -318,9 +317,14 @@ QJsonValue QJsonObject::operator [](const QString &key) const
  */
 QJsonValueRef QJsonObject::operator [](const QString &key)
 {
+    // ### somewhat inefficient, as we lookup the key twice if it doesn't yet exist
     bool keyExists = false;
     int index = o ? o->indexOf(key, &keyExists) : -1;
-    return keyExists ? QJsonValueRef(this, index) : QJsonValueRef(this, key);
+    if (!keyExists) {
+        iterator i = insert(key, QJsonValue());
+        index = i.i;
+    }
+    return QJsonValueRef(this, index);
 }
 
 /*!
@@ -1034,9 +1038,7 @@ void QJsonObject::compact()
 
     detach();
     d->compact();
-    using namespace QJsonPrivate;
+    o = static_cast<QJsonPrivate::Object *>(d->header->root());
-    o = static_cast<Object *>(d->header->root());
-    ObjectUndefinedKeys::removeKeys(this); // ### Qt6 move it to ~QJsonValueRef
 }
 
 /*!
@@ -1073,8 +1075,6 @@ void QJsonObject::setValueAt(int i, const QJsonValue &val)
     insert(e->key(), val);
 }
 
-QBasicMutex QJsonPrivate::ObjectUndefinedKeys::mutex;
-
 #if !defined(QT_NO_DEBUG_STREAM) && !defined(QT_JSON_READONLY)
 QDebug operator<<(QDebug dbg, const QJsonObject &o)
 {

src/corelib/json/qjsonvalue.cpp

@@ -673,10 +673,11 @@ void QJsonValue::detach()
     However, they are not explicitly documented here.
 */
 
+
 QJsonValueRef &QJsonValueRef::operator =(const QJsonValue &val)
 {
     if (is_object)
-        UnionHelper::setValueAt(this, val);
+        o->setValueAt(index, val);
     else
         a->replace(index, val);
 
@@ -686,7 +687,7 @@ QJsonValueRef &QJsonValueRef::operator =(const QJsonValue &val)
 QJsonValueRef &QJsonValueRef::operator =(const QJsonValueRef &ref)
 {
     if (is_object)
-        UnionHelper::setValueAt(this, ref);
+        o->setValueAt(index, ref);
     else
         a->replace(index, ref);
 
@@ -707,7 +708,7 @@ QJsonValue QJsonValueRef::toValue() const
 {
     if (!is_object)
         return a->at(index);
-    return UnionHelper::valueAt(this);
+    return o->valueAt(index);
 }
 
 #if !defined(QT_NO_DEBUG_STREAM) && !defined(QT_JSON_READONLY)

src/corelib/json/qjsonvalue.h

@@ -149,7 +149,6 @@ public:
         : a(array), is_object(false), index(idx) {}
     QJsonValueRef(QJsonObject *object, int idx)
         : o(object), is_object(true), index(idx) {}
-    inline QJsonValueRef(QJsonObject *object, const QString &key);
 
     inline operator QJsonValue() const { return toValue(); }
     QJsonValueRef &operator = (const QJsonValue &val);
@@ -189,7 +188,6 @@ private:
     };
     uint is_object : 1;
     uint index : 31;
-    struct UnionHelper;
 };
 
 #ifndef Q_QDOC

tests/auto/corelib/json/tst_qtjson.cpp

@@ -92,7 +92,6 @@ private Q_SLOTS:
     void nullObject();
     void constNullObject();
 
-    void undefinedKeys();
     void keySorting();
 
     void undefinedValues();
@@ -723,10 +722,6 @@ void tst_QtJson::testObjectIteration()
 {
     QJsonObject object;
 
-    for (QJsonObject::iterator it = object.begin(); it != object.end(); ++it)
-        QVERIFY(false);
-
-    object["undefined"];
     for (QJsonObject::iterator it = object.begin(); it != object.end(); ++it)
         QVERIFY(false);
 
@@ -736,14 +731,6 @@ void tst_QtJson::testObjectIteration()
     for (QJsonObject::iterator it = object.begin(); it != object.end(); ++it)
         QVERIFY(false);
 
-    object.insert(property, 11);
-    object["aaa"]; // before "kkk"
-    object["zzz"]; // after "kkk"
-    for (QJsonObject::iterator it = object.begin(); it != object.end(); ++it)
-        QCOMPARE(it.key(), property);
-
-    object = QJsonObject();
-
     for (int i = 0; i < 10; ++i)
         object[QString::number(i)] = (double)i;
 
@@ -1021,7 +1008,6 @@ void tst_QtJson::nullObject()
     nonNull.insert(QLatin1String("foo"), QLatin1String("bar"));
 
     QCOMPARE(nullObject, QJsonObject());
-    QCOMPARE(nullObject.size(), 0);
     QVERIFY(nullObject != nonNull);
     QVERIFY(nonNull != nullObject);
 
@@ -1039,27 +1025,8 @@ void tst_QtJson::nullObject()
     QCOMPARE(nullObject.keys(), QStringList());
     nullObject.remove("foo");
     QCOMPARE(nullObject, QJsonObject());
-    QCOMPARE(QJsonValue(nullObject["foo"]), QJsonValue(QJsonValue::Undefined));
     QCOMPARE(nullObject.take("foo"), QJsonValue(QJsonValue::Undefined));
     QCOMPARE(nullObject.contains("foo"), false);
-    QCOMPARE(nullObject, QJsonObject());
-    QCOMPARE(nullObject.size(), 0);
-
-    // There is not way to check if internal temporary storage for QJsonValueRef is removed correctly by
-    // remove, take or compaction but at least we should not crash
-    nullObject["foo"];
-    QCOMPARE(nullObject.size(), 0);
-    nullObject.remove("foo");
-    QCOMPARE(nullObject.size(), 0);
-    nullObject["foo"];
-    QCOMPARE(nullObject.size(), 0);
-    nullObject.take("foo");
-    QCOMPARE(nullObject.size(), 0);
-    QString property("foo");
-    for (int i = 0; i < 128; ++i)
-        nullObject[property + QString::number(i)];
-
-    QCOMPARE(nullObject.size(), 0);
 }
 
 void tst_QtJson::constNullObject()
@@ -1079,16 +1046,6 @@ void tst_QtJson::constNullObject()
     QCOMPARE(nullObject["foo"], QJsonValue(QJsonValue::Undefined));
 }
 
-void tst_QtJson::undefinedKeys()
-{
-    QJsonObject null;
-    QVERIFY(null.keys().isEmpty());
-
-    // check that an internal udefined doesn't show up
-    null["undefined"];
-    QVERIFY(null.keys().isEmpty());
-}
-
 void tst_QtJson::keySorting()
 {
     const char *json = "{ \"B\": true, \"A\": false }";
@@ -1224,11 +1181,6 @@ void tst_QtJson::toVariantMap()
     QVariantMap map = object.toVariantMap();
     QVERIFY(map.isEmpty());
 
-    // check an empty object with an internal undefined
-    QJsonValueRef unused = object["undefined"];
-    Q_UNUSED(unused);
-    QVERIFY(object.toVariantMap().isEmpty());
-
     object.insert("Key", QString("Value"));
     object.insert("null", QJsonValue());
     QJsonArray array;
@@ -2430,7 +2382,6 @@ void tst_QtJson::objectEquals_data()
 
     QJsonObject referencedEmpty;
     referencedEmpty["undefined"];
-    QTest::newRow("referenced empty vs default") << referencedEmpty << QJsonObject() << true;
     QTest::newRow("referenced empty vs referenced empty") << referencedEmpty << referencedEmpty << true;
     QTest::newRow("referenced empty vs object") << referencedEmpty << object1 << false;
 
@@ -2443,9 +2394,7 @@ void tst_QtJson::objectEquals_data()
     referencedObject2["zzzzzzzzz"]; // after "property"
     QTest::newRow("referenced object vs default") << referencedObject1 << QJsonObject() << false;
     QTest::newRow("referenced object vs referenced object") << referencedObject1 << referencedObject1 << true;
-    QTest::newRow("referenced object vs object (same)") << referencedObject1 << object1 << true;
     QTest::newRow("referenced object vs object (different)") << referencedObject1 << object3 << false;
-    QTest::newRow("referenced object vs referenced object (same)") << referencedObject1 << referencedObject2 << true;
 }
 
 void tst_QtJson::objectEquals()