Get rid of special handling of var properties

These can be handled in a simple way now by using a special
propertyType value indicating that we have a var property.

Also remove the additional write calls in the different
readProperty implementations. If the stored data doesn't
match, we can simply return the default value directly.

Change-Id: I3823a971df24bd78f0acdc4c0042776277b3c55f
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

5 files changed, 43 insertions, 90 deletions

diff --git a/src/qml/compiler/qqmltypecompiler.cpp b/src/qml/compiler/qqmltypecompiler.cpp
index 307f945d2..80aa617c5 100644
--- a/src/qml/compiler/qqmltypecompiler.cpp
+++ b/src/qml/compiler/qqmltypecompiler.cpp
@@ -780,15 +780,18 @@ bool QQmlPropertyCacheCreator::createMetaObject(int objectIndex, const QmlIR::Ob
     int propertyIdx = 0;
     for (const QmlIR::Property *p = obj->firstProperty(); p; p = p->next, ++propertyIdx) {
 
-        if (p->type == QV4::CompiledData::Property::Alias ||
-            p->type == QV4::CompiledData::Property::Var)
+        if (p->type == QV4::CompiledData::Property::Alias)
             continue;
 
         int propertyType = 0;
         int vmePropertyType = 0;
         quint32 propertyFlags = 0;
 
-        if (p->type < builtinTypeCount) {
+        if (p->type == QV4::CompiledData::Property::Var) {
+            propertyType = QMetaType::QVariant;
+            vmePropertyType = QQmlVMEMetaData::VarPropertyType;
+            propertyFlags = QQmlPropertyData::IsVarProperty;
+        } else if (p->type < builtinTypeCount) {
             propertyType = builtinTypes[p->type].metaType;
             vmePropertyType = propertyType;
 
@@ -852,30 +855,6 @@ bool QQmlPropertyCacheCreator::createMetaObject(int objectIndex, const QmlIR::Ob
         vmd->propertyCount++;
     }
 
-    // Now do var properties
-    propertyIdx = 0;
-    for (const QmlIR::Property *p = obj->firstProperty(); p; p = p->next, ++propertyIdx) {
-
-        if (p->type != QV4::CompiledData::Property::Var)
-            continue;
-
-        quint32 propertyFlags = QQmlPropertyData::IsVarProperty;
-        if (!(p->flags & QV4::CompiledData::Property::IsReadOnly))
-            propertyFlags |= QQmlPropertyData::IsWritable;
-
-        VMD *vmd = (QQmlVMEMetaData *)dynamicData.data();
-        (vmd->propertyData() + vmd->propertyCount)->propertyType = QMetaType::QVariant;
-        vmd->propertyCount++;
-        ((QQmlVMEMetaData *)dynamicData.data())->varPropertyCount++;
-
-        QString propertyName = stringAt(p->nameIndex);
-        if (propertyIdx == obj->indexOfDefaultProperty) cache->_defaultPropertyName = propertyName;
-        cache->appendProperty(propertyName, propertyFlags, effectivePropertyIndex++,
-                              QMetaType::QVariant, effectiveSignalIndex);
-
-        effectiveSignalIndex++;
-    }
-
     // Alias property count.  Actual data is setup in buildDynamicMetaAliases
     ((QQmlVMEMetaData *)dynamicData.data())->aliasCount = aliasCount;
 
diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp
index 08c5ed4f9..ab3303769 100644
--- a/src/qml/qml/qqmlvmemetaobject.cpp
+++ b/src/qml/qml/qqmlvmemetaobject.cpp
@@ -151,7 +151,7 @@ QQmlVMEMetaObject::QQmlVMEMetaObject(QObject *obj,
                                      const QQmlVMEMetaData *meta, QV4::ExecutionContext *qmlBindingContext, QQmlCompiledData *compiledData)
 : object(obj),
   ctxt(QQmlData::get(obj, true)->outerContext), cache(cache), metaData(meta),
-  hasAssignedMetaObjectData(false), aliasEndpoints(0), firstVarPropertyIndex(-1),
+  hasAssignedMetaObjectData(false), aliasEndpoints(0),
   propertiesInitialized(false), interceptors(0), v8methods(0)
 {
     QObjectPrivate *op = QObjectPrivate::get(obj);
@@ -176,7 +176,7 @@ QQmlVMEMetaObject::QQmlVMEMetaObject(QObject *obj,
     bool needsJSWrapper = (metaData->propertyCount > 0);
 
     // ### Optimize
-    for (int ii = 0; ii < metaData->propertyCount - metaData->varPropertyCount; ++ii) {
+    for (int ii = 0; ii < metaData->propertyCount; ++ii) {
         int t = (metaData->propertyData() + ii)->propertyType;
         if (t == list_type) {
             listProperties.append(List(methodOffset() + ii, this));
@@ -186,8 +186,6 @@ QQmlVMEMetaObject::QQmlVMEMetaObject(QObject *obj,
         }
     }
 
-    firstVarPropertyIndex = metaData->propertyCount - metaData->varPropertyCount;
-
     if (needsJSWrapper)
         ensureQObjectWrapper();
 
@@ -317,10 +315,8 @@ int QQmlVMEMetaObject::readPropertyAsInt(int id)
 
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
-    if (!sv->isInt32()) {
-        writeProperty(id, int(0));
+    if (!sv->isInt32())
         return 0;
-    }
     return sv->integerValue();
 }
 
@@ -332,10 +328,8 @@ bool QQmlVMEMetaObject::readPropertyAsBool(int id)
 
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
-    if (!sv->isBoolean()) {
-        writeProperty(id, false);
+    if (!sv->isBoolean())
         return false;
-    }
     return sv->booleanValue();
 }
 
@@ -347,10 +341,8 @@ double QQmlVMEMetaObject::readPropertyAsDouble(int id)
 
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
-    if (!sv->isDouble()) {
-        writeProperty(id, 0.0);
+    if (!sv->isDouble())
         return 0.0;
-    }
     return sv->doubleValue();
 }
 
@@ -362,10 +354,8 @@ QString QQmlVMEMetaObject::readPropertyAsString(int id)
 
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
-    if (!sv->isString()) {
-        writeProperty(id, QString());
+    if (!sv->isString())
         return QString();
-    }
     return sv->stringValue()->toQString();
 }
 
@@ -378,10 +368,8 @@ QUrl QQmlVMEMetaObject::readPropertyAsUrl(int id)
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
     const QV4::VariantObject *v = sv->as<QV4::VariantObject>();
-    if (!v || v->d()->data.type() != QVariant::Url) {
-        writeProperty(id, QUrl());
+    if (!v || v->d()->data.type() != QVariant::Url)
         return QUrl();
-    }
     return v->d()->data.value<QUrl>();
 }
 
@@ -394,10 +382,8 @@ QDate QQmlVMEMetaObject::readPropertyAsDate(int id)
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
     const QV4::VariantObject *v = sv->as<QV4::VariantObject>();
-    if (!v || v->d()->data.type() != QVariant::Date) {
-        writeProperty(id, QDate());
+    if (!v || v->d()->data.type() != QVariant::Date)
         return QDate();
-    }
     return v->d()->data.value<QDate>();
 }
 
@@ -410,10 +396,8 @@ QDateTime QQmlVMEMetaObject::readPropertyAsDateTime(int id)
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
     const QV4::VariantObject *v = sv->as<QV4::VariantObject>();
-    if (!v || v->d()->data.type() != QVariant::DateTime) {
-        writeProperty(id, QDateTime());
+    if (!v || v->d()->data.type() != QVariant::DateTime)
         return QDateTime();
-    }
     return v->d()->data.value<QDateTime>();
 }
 
@@ -426,10 +410,8 @@ QSizeF QQmlVMEMetaObject::readPropertyAsSizeF(int id)
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
     const QV4::VariantObject *v = sv->as<QV4::VariantObject>();
-    if (!v || v->d()->data.type() != QVariant::SizeF) {
-        writeProperty(id, QSizeF());
+    if (!v || v->d()->data.type() != QVariant::SizeF)
         return QSizeF();
-    }
     return v->d()->data.value<QSizeF>();
 }
 
@@ -442,10 +424,8 @@ QPointF QQmlVMEMetaObject::readPropertyAsPointF(int id)
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
     const QV4::VariantObject *v = sv->as<QV4::VariantObject>();
-    if (!v || v->d()->data.type() != QVariant::PointF) {
-        writeProperty(id, QPointF());
+    if (!v || v->d()->data.type() != QVariant::PointF)
         return QPointF();
-    }
     return v->d()->data.value<QPointF>();
 }
 
@@ -458,10 +438,8 @@ QObject* QQmlVMEMetaObject::readPropertyAsQObject(int id)
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
     const QV4::QObjectWrapper *wrapper = sv->as<QV4::QObjectWrapper>();
-    if (!wrapper) {
-        writeProperty(id, static_cast<QObject*>(Q_NULLPTR));
+    if (!wrapper)
         return 0;
-    }
     return wrapper->object();
 }
 
@@ -474,10 +452,8 @@ QRectF QQmlVMEMetaObject::readPropertyAsRectF(int id)
     QV4::Scope scope(properties.engine());
     QV4::ScopedValue sv(scope, *(md->data() + id));
     const QV4::VariantObject *v = sv->as<QV4::VariantObject>();
-    if (!v || v->d()->data.type() != QVariant::RectF) {
-        writeProperty(id, QRectF());
+    if (!v || v->d()->data.type() != QVariant::RectF)
         return QRectF();
-    }
     return v->d()->data.value<QRectF>();
 }
 
@@ -566,8 +542,7 @@ int QQmlVMEMetaObject::metaCall(QMetaObject::Call c, int _id, void **a)
                int t = (metaData->propertyData() + id)->propertyType;
                 bool needActivate = false;
 
-                if (id >= firstVarPropertyIndex) {
-                    Q_ASSERT(t == QMetaType::QVariant);
+                if (t == QQmlVMEMetaData::VarPropertyType) {
                     // the context can be null if accessing var properties from cpp after re-parenting an item.
                     QQmlEnginePrivate *ep = (ctxt == 0 || ctxt->engine == 0) ? 0 : QQmlEnginePrivate::get(ctxt->engine);
                     QV8Engine *v8e = (ep == 0) ? 0 : ep->v8engine();
@@ -858,7 +833,7 @@ QV4::ReturnedValue QQmlVMEMetaObject::method(int index)
 
 QV4::ReturnedValue QQmlVMEMetaObject::readVarProperty(int id)
 {
-    Q_ASSERT(id >= firstVarPropertyIndex);
+    Q_ASSERT((metaData->propertyData() + id)->propertyType == QQmlVMEMetaData::VarPropertyType);
 
     QV4::MemberData *md = propertiesAsMemberData();
     if (md)
@@ -868,30 +843,23 @@ QV4::ReturnedValue QQmlVMEMetaObject::readVarProperty(int id)
 
 QVariant QQmlVMEMetaObject::readPropertyAsVariant(int id)
 {
-    if (id >= firstVarPropertyIndex) {
-        QV4::MemberData *md = propertiesAsMemberData();
-        if (md)
-            return properties.engine()->toVariant(*(md->data() + id), -1);
-    } else {
-        QV4::MemberData *md = propertiesAsMemberData();
-        if (md) {
-            const QV4::QObjectWrapper *wrapper = (md->data() + id)->as<QV4::QObjectWrapper>();
-            if (wrapper)
-                return QVariant::fromValue(wrapper->object());
-            const QV4::VariantObject *v = (md->data() + id)->as<QV4::VariantObject>();
-            if (!v) {
-                writeProperty(id, QVariant());
-                return QVariant();
-            }
+    QV4::MemberData *md = propertiesAsMemberData();
+    if (md) {
+        const QV4::QObjectWrapper *wrapper = (md->data() + id)->as<QV4::QObjectWrapper>();
+        if (wrapper)
+            return QVariant::fromValue(wrapper->object());
+        const QV4::VariantObject *v = (md->data() + id)->as<QV4::VariantObject>();
+        if (v)
             return v->d()->data;
-        }
+        return properties.engine()->toVariant(*(md->data() + id), -1);
     }
     return QVariant();
 }
 
 void QQmlVMEMetaObject::writeVarProperty(int id, const QV4::Value &value)
 {
-    Q_ASSERT(id >= firstVarPropertyIndex);
+    Q_ASSERT((metaData->propertyData() + id)->propertyType == QQmlVMEMetaData::VarPropertyType);
+
     QV4::MemberData *md = propertiesAsMemberData();
     if (!md)
         return;
@@ -930,7 +898,7 @@ void QQmlVMEMetaObject::writeVarProperty(int id, const QV4::Value &value)
 
 void QQmlVMEMetaObject::writeProperty(int id, const QVariant &value)
 {
-    if (id >= firstVarPropertyIndex) {
+    if ((metaData->propertyData() + id)->propertyType == QQmlVMEMetaData::VarPropertyType) {
         QV4::MemberData *md = propertiesAsMemberData();
         if (!md)
             return;
diff --git a/src/qml/qml/qqmlvmemetaobject_p.h b/src/qml/qml/qqmlvmemetaobject_p.h
index 5d4d7055a..3f289c2cc 100644
--- a/src/qml/qml/qqmlvmemetaobject_p.h
+++ b/src/qml/qml/qqmlvmemetaobject_p.h
@@ -72,7 +72,6 @@ QT_BEGIN_NAMESPACE
 
 struct QQmlVMEMetaData
 {
-    short varPropertyCount;
     short propertyCount;
     short aliasCount;
     short signalCount;
@@ -109,6 +108,10 @@ struct QQmlVMEMetaData
         }
     };
 
+    enum {
+        VarPropertyType = -1
+    };
+
     struct PropertyData {
         int propertyType;
     };
@@ -198,7 +201,6 @@ public:
     QQmlVMEMetaObjectEndpoint *aliasEndpoints;
 
     QV4::WeakValue properties;
-    int firstVarPropertyIndex;
     bool propertiesInitialized;
     inline void allocateProperties();
     inline bool ensurePropertiesAllocated();
diff --git a/src/quick/designer/qqmldesignermetaobject.cpp b/src/quick/designer/qqmldesignermetaobject.cpp
index 4273c0fbc..e80012ba3 100644
--- a/src/quick/designer/qqmldesignermetaobject.cpp
+++ b/src/quick/designer/qqmldesignermetaobject.cpp
@@ -75,8 +75,7 @@ struct MetaPropertyData {
 
 static bool constructedMetaData(const QQmlVMEMetaData* data)
 {
-    return data->varPropertyCount == 0
-            && data->propertyCount == 0
+    return data->propertyCount == 0
             && data->aliasCount == 0
             && data->signalCount == 0
             && data->methodCount == 0;
@@ -85,7 +84,6 @@ static bool constructedMetaData(const QQmlVMEMetaData* data)
 static QQmlVMEMetaData* fakeMetaData()
 {
     QQmlVMEMetaData* data = new QQmlVMEMetaData;
-    data->varPropertyCount = 0;
     data->propertyCount = 0;
     data->aliasCount = 0;
     data->signalCount = 0;
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 8bb9ddc07..b524b30e1 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -5007,6 +5007,12 @@ void tst_qqmlecmascript::propertyVarCircular()
     QObject *object = component.create();
     QVERIFY(object != 0);
     QMetaObject::invokeMethod(object, "assignCircular");           // cause assignment and gc
+    {
+        QCOMPARE(object->property("canaryInt"), QVariant(5));
+        QVariant canaryResourceVariant = object->property("canaryResource");
+        QVERIFY(canaryResourceVariant.isValid());
+    }
+
     QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete); // process deleteLater() events from QV8QObjectWrapper.
     QCoreApplication::processEvents();
     QCOMPARE(object->property("canaryInt"), QVariant(5));