QJsonValueConstRef: optimize to{Int,Integer,Bool,Double,String}()

The CBOR-based implementation from Qt 5.15 simply created a QJsonValue and
then called the equivalent function on that result. For integrals and
double where the QJsonValue matched the asked type, the cost was minimal
anyway. For the other types and when the type didn't match, this
resulted in up/down the reference counter in QCborContainerPrivate and
an out-of-line call to the destructor.

This improves the performance for code like:

  for (QJsonValue v : array) {
      if (v.toString() == x) {
          doSomething();
      }
  }

This change propagates the inadviseable behavior of allowing a
dereference of the end() reference in concrete() to concreteType() (and
unit-tests it) but does not do the same for the toXxx() functions. Doing
that causes a dereference of QList's end() iterator, which is UB
(asserts false in debug mode).

Change-Id: I89446ea06b5742efb194fffd16bb3beccd1098e4
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

2 files changed, 100 insertions, 36 deletions

diff --git a/src/corelib/serialization/qjsonvalue.cpp b/src/corelib/serialization/qjsonvalue.cpp
index 481ac64f55..27cad8941e 100644
--- a/src/corelib/serialization/qjsonvalue.cpp
+++ b/src/corelib/serialization/qjsonvalue.cpp
@@ -59,6 +59,29 @@
 
 QT_BEGIN_NAMESPACE
 
+static QJsonValue::Type convertFromCborType(QCborValue::Type type) noexcept
+{
+    switch (type) {
+    case QCborValue::Null:
+        return QJsonValue::Null;
+    case QCborValue::True:
+    case QCborValue::False:
+        return QJsonValue::Bool;
+    case QCborValue::Double:
+    case QCborValue::Integer:
+        return QJsonValue::Double;
+    case QCborValue::String:
+        return QJsonValue::String;
+    case QCborValue::Array:
+        return QJsonValue::Array;
+    case QCborValue::Map:
+        return QJsonValue::Object;
+    case QCborValue::Undefined:
+    default:
+        return QJsonValue::Undefined;
+    }
+}
+
 /*!
     \class QJsonValue
     \inmodule QtCore
@@ -608,25 +631,7 @@ QVariant QJsonValue::toVariant() const
  */
 QJsonValue::Type QJsonValue::type() const
 {
-    switch (value.type()) {
-    case QCborValue::Null:
-        return QJsonValue::Null;
-    case QCborValue::True:
-    case QCborValue::False:
-        return QJsonValue::Bool;
-    case QCborValue::Double:
-    case QCborValue::Integer:
-        return QJsonValue::Double;
-    case QCborValue::String:
-        return QJsonValue::String;
-    case QCborValue::Array:
-        return QJsonValue::Array;
-    case QCborValue::Map:
-        return QJsonValue::Object;
-    case QCborValue::Undefined:
-    default:
-        return QJsonValue::Undefined;
-    }
+    return convertFromCborType(value.type());
 }
 
 /*!
@@ -950,6 +955,56 @@ QJsonObject QJsonValueConstRef::toObject() const
     return concrete(*this).toObject();
 }
 
+QJsonValue::Type QJsonValueConstRef::concreteType(QJsonValueConstRef self) noexcept
+{
+    return convertFromCborType(QJsonPrivate::Value::elementHelper(self).type);
+}
+
+bool QJsonValueConstRef::concreteBool(QJsonValueConstRef self, bool defaultValue) noexcept
+{
+    auto &e = QJsonPrivate::Value::elementHelper(self);
+    if (e.type == QCborValue::False)
+        return false;
+    if (e.type == QCborValue::True)
+        return true;
+    return defaultValue;
+}
+
+qint64 QJsonValueConstRef::concreteInt(QJsonValueConstRef self, qint64 defaultValue, bool clamp) noexcept
+{
+    auto &e = QJsonPrivate::Value::elementHelper(self);
+    qint64 v = defaultValue;
+    if (e.type == QCborValue::Double) {
+        // convertDoubleTo modifies the output even on returning false
+        if (!convertDoubleTo<qint64>(e.fpvalue(), &v))
+            v = defaultValue;
+    } else if (e.type == QCborValue::Integer) {
+        v = e.value;
+    }
+    if (clamp && qint64(int(v)) != v)
+        return defaultValue;
+    return v;
+}
+
+double QJsonValueConstRef::concreteDouble(QJsonValueConstRef self, double defaultValue) noexcept
+{
+    auto &e = QJsonPrivate::Value::elementHelper(self);
+    if (e.type == QCborValue::Double)
+        return e.fpvalue();
+    if (e.type == QCborValue::Integer)
+        return e.value;
+    return defaultValue;
+}
+
+QString QJsonValueConstRef::concreteString(QJsonValueConstRef self, const QString &defaultValue)
+{
+    const QCborContainerPrivate *d = QJsonPrivate::Value::container(self);
+    qsizetype index = QJsonPrivate::Value::indexHelper(self);
+    if (d->elements.at(index).type != QCborValue::String)
+        return defaultValue;
+    return d->stringAt(index);
+}
+
 QJsonValue QJsonValueConstRef::concrete(QJsonValueConstRef self) noexcept
 {
     const QCborContainerPrivate *d = QJsonPrivate::Value::container(self);
diff --git a/src/corelib/serialization/qjsonvalue.h b/src/corelib/serialization/qjsonvalue.h
index 11b9ad1b01..4f9c3af578 100644
--- a/src/corelib/serialization/qjsonvalue.h
+++ b/src/corelib/serialization/qjsonvalue.h
@@ -160,7 +160,7 @@ public:
     inline operator QJsonValue() const { return concrete(*this); }
 
     Q_CORE_EXPORT QVariant toVariant() const;
-    QJsonValue::Type type() const { return concrete(*this).type(); }
+    QJsonValue::Type type() const { return concreteType(*this); }
     bool isNull() const { return type() == QJsonValue::Null; }
     bool isBool() const { return type() == QJsonValue::Bool; }
     bool isDouble() const { return type() == QJsonValue::Double; }
@@ -170,15 +170,15 @@ public:
     bool isUndefined() const { return type() == QJsonValue::Undefined; }
 
     bool toBool(bool defaultValue = false) const
-    { return concrete(*this).toBool(defaultValue); }
+    { return concreteBool(*this, defaultValue); }
     int toInt(int defaultValue = 0) const
-    { return concrete(*this).toInt(defaultValue); }
+    { return concreteInt(*this, defaultValue, true); }
     qint64 toInteger(qint64 defaultValue = 0) const
-    { return concrete(*this).toInteger(defaultValue); }
+    { return concreteInt(*this, defaultValue, false); }
     double toDouble(double defaultValue = 0) const
-    { return concrete(*this).toDouble(defaultValue); }
+    { return concreteDouble(*this, defaultValue); }
     QString toString(const QString &defaultValue = {}) const
-    { return concrete(*this).toString(defaultValue); }
+    { return concreteString(*this, defaultValue); }
     Q_CORE_EXPORT QJsonArray toArray() const;
     Q_CORE_EXPORT QJsonObject toObject() const;
 
@@ -195,6 +195,15 @@ protected:
     QJsonValueConstRef(QJsonObject *object, qsizetype idx)
         : o(object), is_object(true), index(static_cast<quint64>(idx)) {}
 
+    Q_CORE_EXPORT static QJsonValue::Type
+    concreteType(QJsonValueConstRef self) noexcept Q_DECL_PURE_FUNCTION;
+    Q_CORE_EXPORT static bool
+    concreteBool(QJsonValueConstRef self, bool defaultValue) noexcept Q_DECL_PURE_FUNCTION;
+    Q_CORE_EXPORT static qint64
+    concreteInt(QJsonValueConstRef self, qint64 defaultValue, bool clamp) noexcept Q_DECL_PURE_FUNCTION;
+    Q_CORE_EXPORT static double
+    concreteDouble(QJsonValueConstRef self, double defaultValue) noexcept Q_DECL_PURE_FUNCTION;
+    Q_CORE_EXPORT static QString concreteString(QJsonValueConstRef self, const QString &defaultValue);
     Q_CORE_EXPORT static QJsonValue concrete(QJsonValueConstRef self) noexcept;
 
     union {
@@ -228,7 +237,7 @@ public:
     operator QJsonValue() const { return toValue(); }
 
     QVariant toVariant() const;
-    inline QJsonValue::Type type() const { return toValue().type(); }
+    inline QJsonValue::Type type() const { return QJsonValueConstRef::type(); }
     inline bool isNull() const { return type() == QJsonValue::Null; }
     inline bool isBool() const { return type() == QJsonValue::Bool; }
     inline bool isDouble() const { return type() == QJsonValue::Double; }
@@ -237,20 +246,20 @@ public:
     inline bool isObject() const { return type() == QJsonValue::Object; }
     inline bool isUndefined() const { return type() == QJsonValue::Undefined; }
 
-    inline bool toBool(bool defaultValue = false) const { return toValue().toBool(defaultValue); }
-    inline int toInt(int defaultValue = 0) const { return toValue().toInt(defaultValue); }
-    inline qint64 toInteger(qint64 defaultValue = 0) const { return toValue().toInteger(defaultValue); }
-    inline double toDouble(double defaultValue = 0) const { return toValue().toDouble(defaultValue); }
-    inline QString toString(const QString &defaultValue = {}) const { return toValue().toString(defaultValue); }
+    inline bool toBool(bool defaultValue = false) const { return QJsonValueConstRef::toBool(defaultValue); }
+    inline int toInt(int defaultValue = 0) const { return QJsonValueConstRef::toInt(defaultValue); }
+    inline qint64 toInteger(qint64 defaultValue = 0) const { return QJsonValueConstRef::toInteger(defaultValue); }
+    inline double toDouble(double defaultValue = 0) const { return QJsonValueConstRef::toDouble(defaultValue); }
+    inline QString toString(const QString &defaultValue = {}) const { return QJsonValueConstRef::toString(defaultValue); }
     QJsonArray toArray() const;
     QJsonObject toObject() const;
 
-    const QJsonValue operator[](QStringView key) const { return toValue()[key]; }
-    const QJsonValue operator[](QLatin1String key) const { return toValue()[key]; }
-    const QJsonValue operator[](qsizetype i) const { return toValue()[i]; }
+    const QJsonValue operator[](QStringView key) const { return QJsonValueConstRef::operator[](key); }
+    const QJsonValue operator[](QLatin1String key) const { return QJsonValueConstRef::operator[](key); }
+    const QJsonValue operator[](qsizetype i) const { return QJsonValueConstRef::operator[](i); }
 
-    inline bool operator==(const QJsonValue &other) const { return toValue() == other; }
-    inline bool operator!=(const QJsonValue &other) const { return toValue() != other; }
+    inline bool operator==(const QJsonValue &other) const { return QJsonValueConstRef::operator==(other); }
+    inline bool operator!=(const QJsonValue &other) const { return QJsonValueConstRef::operator!=(other); }
 
 private:
     QJsonValue toValue() const;