Reading QJsonObject property should not modify the object itself.

Before this change such code:

    QJsonObject o;
    o["blah"];

would create property "blah" and assign null value to it, while
this code:

    const QJsonObject o;
    o["blah"];

would not. The change unifies the confusing behavior. Now reading
a non-existing property, is not causing a property to be added
in any visible way.

Internally QJsonObject stores a special hash of undefined, but
referenced values. Such reference is supposed to not live long,
only to the first compacting or assignment.

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

4 files changed, 83 insertions, 11 deletions

diff --git a/src/corelib/json/qjson_p.h b/src/corelib/json/qjson_p.h
index 7d0162938d..92330175ba 100644
--- a/src/corelib/json/qjson_p.h
+++ b/src/corelib/json/qjson_p.h
@@ -61,6 +61,8 @@
 #include <qstring.h>
 #include <qendian.h>
 #include <qnumeric.h>
+#include <QtCore/qhash.h>
+#include <QtCore/qmutex.h>
 
 #include <limits.h>
 #include <limits>
@@ -776,6 +778,75 @@ 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
diff --git a/src/corelib/json/qjsonobject.cpp b/src/corelib/json/qjsonobject.cpp
index fb975c65e0..94ec9a2818 100644
--- a/src/corelib/json/qjsonobject.cpp
+++ b/src/corelib/json/qjsonobject.cpp
@@ -134,6 +134,7 @@ QJsonObject::~QJsonObject()
 {
     if (d && !d->ref.deref())
         delete d;
+    QJsonPrivate::ObjectUndefinedKeys::removeKeys(this); // ### Qt6 move it to ~QJsonValueRef
 }
 
 /*!
@@ -290,14 +291,9 @@ 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;
-    if (!keyExists) {
-        iterator i = insert(key, QJsonValue());
-        index = i.i;
-    }
-    return QJsonValueRef(this, index);
+    return keyExists ? QJsonValueRef(this, index) : QJsonValueRef(this, key);
 }
 
 /*!
@@ -1001,7 +997,9 @@ void QJsonObject::compact()
 
     detach();
     d->compact();
-    o = static_cast<QJsonPrivate::Object *>(d->header->root());
+    using namespace QJsonPrivate;
+    o = static_cast<Object *>(d->header->root());
+    ObjectUndefinedKeys::removeKeys(this); // ### Qt6 move it to ~QJsonValueRef
 }
 
 /*!
@@ -1038,6 +1036,8 @@ 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)
 {
diff --git a/src/corelib/json/qjsonvalue.cpp b/src/corelib/json/qjsonvalue.cpp
index 4c4838d314..1224a24709 100644
--- a/src/corelib/json/qjsonvalue.cpp
+++ b/src/corelib/json/qjsonvalue.cpp
@@ -669,11 +669,10 @@ void QJsonValue::detach()
     However, they are not explicitly documented here.
 */
 
-
 QJsonValueRef &QJsonValueRef::operator =(const QJsonValue &val)
 {
     if (is_object)
-        o->setValueAt(index, val);
+        UnionHelper::setValueAt(this, val);
     else
         a->replace(index, val);
 
@@ -683,7 +682,7 @@ QJsonValueRef &QJsonValueRef::operator =(const QJsonValue &val)
 QJsonValueRef &QJsonValueRef::operator =(const QJsonValueRef &ref)
 {
     if (is_object)
-        o->setValueAt(index, ref);
+        UnionHelper::setValueAt(this, ref);
     else
         a->replace(index, ref);
 
@@ -704,7 +703,7 @@ QJsonValue QJsonValueRef::toValue() const
 {
     if (!is_object)
         return a->at(index);
-    return o->valueAt(index);
+    return UnionHelper::valueAt(this);
 }
 
 #if !defined(QT_NO_DEBUG_STREAM) && !defined(QT_JSON_READONLY)
diff --git a/src/corelib/json/qjsonvalue.h b/src/corelib/json/qjsonvalue.h
index 97e20b574d..a00bc0b72f 100644
--- a/src/corelib/json/qjsonvalue.h
+++ b/src/corelib/json/qjsonvalue.h
@@ -149,6 +149,7 @@ 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);
@@ -188,6 +189,7 @@ private:
     };
     uint is_object : 1;
     uint index : 31;
+    struct UnionHelper;
 };
 
 #if !defined(QT_NO_DEBUG_STREAM) && !defined(QT_JSON_READONLY)