Fixup the implementation mess for QJSValue(QString)

Until now we were using a QV4::String without engine to
represent this case. But this leads to lots of quirks, where
we ended up trying to access the engine (or the internalclass/vtable)
of this string anyway.

Now just represent it by using an QString in QJSValuePrivate, and use
an empty value to represent it. This adds a little bit of code to
QJSValue and QJSEngine, but is more stable and maintainable in the
longer term.

Change-Id: I3358165ee64e788274225743a95dfb13346225cc
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

3 files changed, 111 insertions, 10 deletions

diff --git a/src/qml/jsapi/qjsengine.cpp b/src/qml/jsapi/qjsengine.cpp
index 5d8a0202fa..cb050c2518 100644
--- a/src/qml/jsapi/qjsengine.cpp
+++ b/src/qml/jsapi/qjsengine.cpp
@@ -49,6 +49,7 @@
 #include "private/qv4mm_p.h"
 #include "private/qv4globalobject_p.h"
 #include "private/qv4script_p.h"
+#include "private/qv4runtime_p.h"
 
 #include <QtCore/qdatetime.h>
 #include <QtCore/qmetaobject.h>
@@ -376,6 +377,54 @@ bool QJSEngine::convertV2(const QJSValue &value, int type, void *ptr)
         QV4::Scope scope(engine->m_v4Engine);
         QV4::ScopedValue v(scope, vp->getValue(engine->m_v4Engine));
         return engine->metaTypeFromJS(v, type, ptr);
+    } else if (vp->value.isEmpty()) {
+        // have a string based value without engine. Do conversion manually
+        if (type == QMetaType::Bool) {
+            *reinterpret_cast<bool*>(ptr) = vp->string.length() != 0;
+            return true;
+        }
+        if (type == QMetaType::QString) {
+            *reinterpret_cast<QString*>(ptr) = vp->string;
+            return true;
+        }
+        double d = QV4::__qmljs_string_to_number(vp->string);
+        switch (type) {
+            case QMetaType::Int:
+                *reinterpret_cast<int*>(ptr) = QV4::Primitive::toInt32(d);
+                return true;
+            case QMetaType::UInt:
+                *reinterpret_cast<uint*>(ptr) = QV4::Primitive::toUInt32(d);
+                return true;
+            case QMetaType::LongLong:
+                *reinterpret_cast<qlonglong*>(ptr) = QV4::Primitive::toInteger(d);
+                return true;
+            case QMetaType::ULongLong:
+                *reinterpret_cast<qulonglong*>(ptr) = QV4::Primitive::toInteger(d);
+                return true;
+            case QMetaType::Double:
+                *reinterpret_cast<double*>(ptr) = d;
+                return true;
+            case QMetaType::Float:
+                *reinterpret_cast<float*>(ptr) = d;
+                return true;
+            case QMetaType::Short:
+                *reinterpret_cast<short*>(ptr) = QV4::Primitive::toInt32(d);
+                return true;
+            case QMetaType::UShort:
+                *reinterpret_cast<unsigned short*>(ptr) = QV4::Primitive::toUInt32(d);
+                return true;
+            case QMetaType::Char:
+                *reinterpret_cast<char*>(ptr) = QV4::Primitive::toInt32(d);
+                return true;
+            case QMetaType::UChar:
+                *reinterpret_cast<unsigned char*>(ptr) = QV4::Primitive::toUInt32(d);
+                return true;
+            case QMetaType::QChar:
+                *reinterpret_cast<QChar*>(ptr) = QV4::Primitive::toUInt32(d);
+                return true;
+            default:
+                return false;
+        }
     } else {
         switch (type) {
             case QMetaType::Bool:
diff --git a/src/qml/jsapi/qjsvalue.cpp b/src/qml/jsapi/qjsvalue.cpp
index 4035fb9fa6..f0e92ebd4d 100644
--- a/src/qml/jsapi/qjsvalue.cpp
+++ b/src/qml/jsapi/qjsvalue.cpp
@@ -58,20 +58,22 @@
 
 QV4::ReturnedValue QJSValuePrivate::getValue(QV4::ExecutionEngine *e)
 {
-    if (!this->engine)
+    if (!this->engine) {
         this->engine = e;
-    else if (this->engine != e) {
+    } else if (this->engine != e) {
         qWarning("JSValue can't be reassigned to another engine.");
         return QV4::Encode::undefined();
     }
-    if (value.asString() == &string) {
-        value = QV4::Encode(engine->newString(string.toQString()));
+
+    if (value.isEmpty()) {
+        value = QV4::Encode(engine->newString(string));
         PersistentValuePrivate **listRoot = &engine->memoryManager->m_persistentValues;
         prev = listRoot;
         next = *listRoot;
         *prev = this;
         if (next)
             next->prev = &this->next;
+        string = QString();
     }
     return value.asReturnedValue();
 }
@@ -210,7 +212,7 @@ QJSValue::QJSValue(const QLatin1String &value)
 */
 #ifndef QT_NO_CAST_FROM_ASCII
 QJSValue::QJSValue(const char *value)
-    : d(new QJSValuePrivate(QString::fromLatin1(value)))
+    : d(new QJSValuePrivate(QString::fromUtf8(value)))
 {
 }
 #endif
@@ -275,7 +277,7 @@ bool QJSValue::isNull() const
 */
 bool QJSValue::isString() const
 {
-    return d->value.isString();
+    return d->value.isEmpty() || d->value.isString();
 }
 
 /*!
@@ -359,6 +361,8 @@ bool QJSValue::isVariant() const
 */
 QString QJSValue::toString() const
 {
+    if (d->value.isEmpty())
+        return d->string;
     return d->value.toQStringNoThrow();
 }
 
@@ -376,6 +380,9 @@ QString QJSValue::toString() const
 */
 double QJSValue::toNumber() const
 {
+    if (d->value.isEmpty())
+        return __qmljs_string_to_number(d->string);
+
     QV4::ExecutionContext *ctx = d->engine ? d->engine->current : 0;
     double dbl = d->value.toNumber();
     if (ctx && ctx->engine->hasException) {
@@ -399,6 +406,9 @@ double QJSValue::toNumber() const
 */
 bool QJSValue::toBool() const
 {
+    if (d->value.isEmpty())
+        return d->string.length() > 0;
+
     QV4::ExecutionContext *ctx = d->engine ? d->engine->current : 0;
     bool b = d->value.toBoolean();
     if (ctx && ctx->engine->hasException) {
@@ -422,6 +432,9 @@ bool QJSValue::toBool() const
 */
 qint32 QJSValue::toInt() const
 {
+    if (d->value.isEmpty())
+        return QV4::Primitive::toInt32(__qmljs_string_to_number(d->string));
+
     QV4::ExecutionContext *ctx = d->engine ? d->engine->current : 0;
     qint32 i = d->value.toInt32();
     if (ctx && ctx->engine->hasException) {
@@ -445,6 +458,9 @@ qint32 QJSValue::toInt() const
 */
 quint32 QJSValue::toUInt() const
 {
+    if (d->value.isEmpty())
+        return QV4::Primitive::toUInt32(__qmljs_string_to_number(d->string));
+
     QV4::ExecutionContext *ctx = d->engine ? d->engine->current : 0;
     quint32 u = d->value.toUInt32();
     if (ctx && ctx->engine->hasException) {
@@ -478,6 +494,9 @@ quint32 QJSValue::toUInt() const
 */
 QVariant QJSValue::toVariant() const
 {
+    if (d->value.isEmpty())
+        return QVariant(d->string);
+
     return QV4::VariantObject::toVariant(d->value);
 }
 
@@ -720,6 +739,22 @@ QJSValue& QJSValue::operator=(const QJSValue& other)
     return *this;
 }
 
+static bool js_equal(const QString &string, QV4::ValueRef value)
+{
+    if (value->isString())
+        return string == value->stringValue()->toQString();
+    if (value->isNumber())
+        return __qmljs_string_to_number(string) == value->asDouble();
+    if (value->isBoolean())
+        return __qmljs_string_to_number(string) == value->booleanValue();
+    if (value->isObject()) {
+        Scope scope(value->objectValue()->engine());
+        ScopedValue p(scope, __qmljs_to_primitive(value, PREFERREDTYPE_HINT));
+        return js_equal(string, p);
+    }
+    return false;
+}
+
 /*!
   Returns true if this QJSValue is equal to \a other, otherwise
   returns false. The comparison follows the behavior described in
@@ -746,6 +781,14 @@ QJSValue& QJSValue::operator=(const QJSValue& other)
 */
 bool QJSValue::equals(const QJSValue& other) const
 {
+    if (d->value.isEmpty()) {
+        if (other.d->value.isEmpty())
+            return d->string == other.d->string;
+        return js_equal(d->string, QV4::ValueRef(other.d->value));
+    }
+    if (other.d->value.isEmpty())
+        return other.equals(*this);
+
     return __qmljs_cmp_eq(QV4::ValueRef(d), QV4::ValueRef(other.d));
 }
 
@@ -773,6 +816,16 @@ bool QJSValue::equals(const QJSValue& other) const
 */
 bool QJSValue::strictlyEquals(const QJSValue& other) const
 {
+    if (d->value.isEmpty()) {
+        if (other.d->value.isEmpty())
+            return d->string == other.d->string;
+        if (other.d->value.isString())
+            return d->string == other.d->value.stringValue()->toQString();
+        return false;
+    }
+    if (other.d->value.isEmpty())
+        return other.strictlyEquals(*this);
+
     return __qmljs_strict_equal(QV4::ValueRef(d), QV4::ValueRef(other.d));
 }
 
diff --git a/src/qml/jsapi/qjsvalue_p.h b/src/qml/jsapi/qjsvalue_p.h
index bf839a6f1f..d8da664cc6 100644
--- a/src/qml/jsapi/qjsvalue_p.h
+++ b/src/qml/jsapi/qjsvalue_p.h
@@ -80,17 +80,16 @@ public:
         Q_ASSERT(!value.isEmpty());
     }
     QJSValuePrivate(const QString &s)
-        : PersistentValuePrivate(QV4::Encode::undefined())
-        , string(0, s)
+        : PersistentValuePrivate(QV4::Primitive::emptyValue().asReturnedValue())
+        , string(s)
     {
-        value.val = QV4::Encode(string.asReturned<QV4::String>());
     }
 
     QV4::ReturnedValue getValue(QV4::ExecutionEngine *e);
 
     static QJSValuePrivate *get(const QJSValue &v) { return v.d; }
 
-    QV4::String string;
+    QString string;
 };
 
 QT_END_NAMESPACE