Clean up property dependency data structures

As a follow-up to the previous commit, this patch cleans up the data structures
used to track dependencies of QML binding expressions and functions to context
and scope properties, determined at compile time.

Instead of "collecting" these depending properties upfront (codegen time), we
propagate the information that a property is a context or scope property into
the IR at codegen time and later in the isel collect these properties and their
notify signal index in a hash in the IR functions. The CompileData structure
generator then can read these hashes directly when writing out the dependency
information.

Change-Id: I32134706e2d24bf63d1b1abad0259ab072460173
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

7 files changed, 87 insertions, 141 deletions

diff --git a/src/qml/compiler/qqmlcodegenerator.cpp b/src/qml/compiler/qqmlcodegenerator.cpp
index 8693c32c92..1b96bad94f 100644
--- a/src/qml/compiler/qqmlcodegenerator.cpp
+++ b/src/qml/compiler/qqmlcodegenerator.cpp
@@ -1364,15 +1364,14 @@ static V4IR::Type resolveQmlType(QQmlEnginePrivate *qmlEngine, V4IR::MemberExpre
             bool ok = false;
             int value = type->enumValue(*member->name, &ok);
             if (ok) {
-                member->memberIsEnum = true;
-                member->enumValue = value;
+                member->setEnumValue(value);
                 resolver->clear();
                 return V4IR::SInt32Type;
             }
         } else if (const QMetaObject *attachedMeta = type->attachedPropertiesType()) {
             QQmlPropertyCache *cache = qmlEngine->cache(attachedMeta);
             initMetaObjectResolver(resolver, cache);
-            member->attachedPropertiesId = type->attachedPropertiesId();
+            member->setAttachedPropertiesId(type->attachedPropertiesId());
             return resolver->resolveMember(qmlEngine, resolver, member);
         }
     }
@@ -1439,8 +1438,7 @@ static V4IR::Type resolveMetaObjectProperty(QQmlEnginePrivate *qmlEngine, V4IR::
             bool ok;
             int value = metaEnum.keyToValue(enumName.constData(), &ok);
             if (ok) {
-                member->memberIsEnum = true;
-                member->enumValue = value;
+                member->setEnumValue(value);
                 resolver->clear();
                 return V4IR::SInt32Type;
             }
@@ -1594,11 +1592,9 @@ V4IR::Expr *JSCodeGen::fallbackNameLookup(const QString &name, int line, int col
         if (propertyExistsButForceNameLookup)
             return 0;
         if (pd) {
-            if (!pd->isConstant())
-                _function->scopeObjectDependencyCandidates.insert(pd); // We don't know if we'll ever read from there or just write, hence candidate
             V4IR::Temp *base = _block->TEMP(_scopeObjectTemp);
             initMetaObjectResolver(&base->memberResolver, _scopeObject);
-            return _block->MEMBER(base, _function->newString(name), pd);
+            return _block->MEMBER(base, _function->newString(name), pd, V4IR::Member::MemberOfQmlScopeObject);
         }
     }
 
@@ -1608,11 +1604,9 @@ V4IR::Expr *JSCodeGen::fallbackNameLookup(const QString &name, int line, int col
         if (propertyExistsButForceNameLookup)
             return 0;
         if (pd) {
-            if (!pd->isConstant())
-                _function->contextObjectDependencyCandidates.insert(pd); // We don't know if we'll ever read from there or just write, hence candidate
             V4IR::Temp *base = _block->TEMP(_contextObjectTemp);
             initMetaObjectResolver(&base->memberResolver, _contextObject);
-            return _block->MEMBER(base, _function->newString(name), pd);
+            return _block->MEMBER(base, _function->newString(name), pd, V4IR::Member::MemberOfQmlContextObject);
         }
     }
 
diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp
index cb17b86702..9041b04837 100644
--- a/src/qml/compiler/qv4compiler.cpp
+++ b/src/qml/compiler/qv4compiler.cpp
@@ -170,20 +170,6 @@ QV4::CompiledData::Unit *QV4::Compiler::JSUnitGenerator::generateUnit(int *total
             registerString(*f->formals.at(i));
         for (int i = 0; i < f->locals.size(); ++i)
             registerString(*f->locals.at(i));
-
-        if (f->hasQmlDependencies()) {
-            QSet<int> idObjectDeps = f->idObjectDependencies;
-            QSet<QQmlPropertyData*> contextPropertyDeps = f->contextObjectDependencies;
-            QSet<QQmlPropertyData*> scopePropertyDeps = f->scopeObjectDependencies;
-
-            if (!idObjectDeps.isEmpty())
-                qmlIdObjectDependenciesPerFunction.insert(f, idObjectDeps);
-            if (!contextPropertyDeps.isEmpty())
-                qmlContextPropertyDependenciesPerFunction.insert(f, contextPropertyDeps);
-            if (!scopePropertyDeps.isEmpty())
-                qmlScopePropertyDependenciesPerFunction.insert(f, scopePropertyDeps);
-        }
-
     }
 
     int unitSize = QV4::CompiledData::Unit::calculateSize(headerSize, strings.size(), irModule->functions.size(), regexps.size(),
@@ -199,23 +185,8 @@ QV4::CompiledData::Unit *QV4::Compiler::JSUnitGenerator::generateUnit(int *total
         if (lineNumberMapping != lineNumberMappingsPerFunction.constEnd())
             lineNumberMappingCount = lineNumberMapping->count() / 2;
 
-        int qmlIdDepsCount = 0;
-        int qmlPropertyDepsCount = 0;
-
-        if (f->hasQmlDependencies()) {
-            IdDependencyHash::ConstIterator idIt = qmlIdObjectDependenciesPerFunction.find(f);
-            if (idIt != qmlIdObjectDependenciesPerFunction.constEnd())
-                qmlIdDepsCount += idIt->count();
-
-            PropertyDependencyHash::ConstIterator it = qmlContextPropertyDependenciesPerFunction.find(f);
-            if (it != qmlContextPropertyDependenciesPerFunction.constEnd())
-                qmlPropertyDepsCount += it->count();
-
-            it = qmlScopePropertyDependenciesPerFunction.find(f);
-            if (it != qmlScopePropertyDependenciesPerFunction.constEnd())
-                qmlPropertyDepsCount += it->count();
-        }
-
+        const int qmlIdDepsCount = f->idObjectDependencies.count();
+        const int qmlPropertyDepsCount = f->scopeObjectPropertyDependencies.count() + f->contextObjectPropertyDependencies.count();
         functionDataSize += QV4::CompiledData::Function::calculateSize(f->formals.size(), f->locals.size(), f->nestedFunctions.size(), lineNumberMappingCount, qmlIdDepsCount, qmlPropertyDepsCount);
     }
 
@@ -353,32 +324,22 @@ int QV4::Compiler::JSUnitGenerator::writeFunction(char *f, int index, QQmlJS::V4
     function->nDependingContextProperties = 0;
     function->nDependingScopeProperties = 0;
 
-    QSet<int> qmlIdObjectDeps;
-    QSet<QQmlPropertyData*> qmlContextPropertyDeps;
-    QSet<QQmlPropertyData*> qmlScopePropertyDeps;
-
-    if (irFunction->hasQmlDependencies()) {
-        qmlIdObjectDeps = qmlIdObjectDependenciesPerFunction.value(irFunction);
-        qmlContextPropertyDeps = qmlContextPropertyDependenciesPerFunction.value(irFunction);
-        qmlScopePropertyDeps = qmlScopePropertyDependenciesPerFunction.value(irFunction);
-
-        if (!qmlIdObjectDeps.isEmpty()) {
-            function->nDependingIdObjects = qmlIdObjectDeps.count();
-            function->dependingIdObjectsOffset = currentOffset;
-            currentOffset += function->nDependingIdObjects * sizeof(quint32);
-        }
-
-        if (!qmlContextPropertyDeps.isEmpty()) {
-            function->nDependingContextProperties = qmlContextPropertyDeps.count();
-            function->dependingContextPropertiesOffset = currentOffset;
-            currentOffset += function->nDependingContextProperties * sizeof(quint32) * 2;
-        }
-
-        if (!qmlScopePropertyDeps.isEmpty()) {
-            function->nDependingScopeProperties = qmlScopePropertyDeps.count();
-            function->dependingScopePropertiesOffset = currentOffset;
-            currentOffset += function->nDependingScopeProperties * sizeof(quint32) * 2;
-        }
+    if (!irFunction->idObjectDependencies.isEmpty()) {
+        function->nDependingIdObjects = irFunction->idObjectDependencies.count();
+        function->dependingIdObjectsOffset = currentOffset;
+        currentOffset += function->nDependingIdObjects * sizeof(quint32);
+    }
+
+    if (!irFunction->contextObjectPropertyDependencies.isEmpty()) {
+        function->nDependingContextProperties = irFunction->contextObjectPropertyDependencies.count();
+        function->dependingContextPropertiesOffset = currentOffset;
+        currentOffset += function->nDependingContextProperties * sizeof(quint32) * 2;
+    }
+
+    if (!irFunction->scopeObjectPropertyDependencies.isEmpty()) {
+        function->nDependingScopeProperties = irFunction->scopeObjectPropertyDependencies.count();
+        function->dependingScopePropertiesOffset = currentOffset;
+        currentOffset += function->nDependingScopeProperties * sizeof(quint32) * 2;
     }
 
     function->location.line = irFunction->line;
@@ -407,19 +368,21 @@ int QV4::Compiler::JSUnitGenerator::writeFunction(char *f, int index, QQmlJS::V4
 
     // write QML dependencies
     quint32 *writtenDeps = (quint32 *)(f + function->dependingIdObjectsOffset);
-    foreach (int id, qmlIdObjectDeps)
+    foreach (int id, irFunction->idObjectDependencies)
         *writtenDeps++ = id;
 
     writtenDeps = (quint32 *)(f + function->dependingContextPropertiesOffset);
-    foreach (QQmlPropertyData *property, qmlContextPropertyDeps) {
-        *writtenDeps++ = property->coreIndex;
-        *writtenDeps++ = property->notifyIndex;
+    for (QQmlJS::V4IR::PropertyDependencyMap::ConstIterator property = irFunction->contextObjectPropertyDependencies.constBegin(), end = irFunction->contextObjectPropertyDependencies.constEnd();
+         property != end; ++property) {
+        *writtenDeps++ = property.key(); // property index
+        *writtenDeps++ = property.value(); // notify index
     }
 
     writtenDeps = (quint32 *)(f + function->dependingScopePropertiesOffset);
-    foreach (QQmlPropertyData *property, qmlScopePropertyDeps) {
-        *writtenDeps++ = property->coreIndex;
-        *writtenDeps++ = property->notifyIndex;
+    for (QQmlJS::V4IR::PropertyDependencyMap::ConstIterator property = irFunction->scopeObjectPropertyDependencies.constBegin(), end = irFunction->scopeObjectPropertyDependencies.constEnd();
+         property != end; ++property) {
+        *writtenDeps++ = property.key(); // property index
+        *writtenDeps++ = property.value(); // notify index
     }
 
     return CompiledData::Function::calculateSize(function->nFormals, function->nLocals, function->nInnerFunctions, function->nLineNumberMappingEntries,
diff --git a/src/qml/compiler/qv4compiler_p.h b/src/qml/compiler/qv4compiler_p.h
index 6338babb5a..1596fcb622 100644
--- a/src/qml/compiler/qv4compiler_p.h
+++ b/src/qml/compiler/qv4compiler_p.h
@@ -94,13 +94,6 @@ struct Q_QML_EXPORT JSUnitGenerator {
     QList<QList<CompiledData::JSClassMember> > jsClasses;
     uint jsClassDataSize;
     uint headerSize;
-
-    typedef QHash<QQmlJS::V4IR::Function *, QSet<int> > IdDependencyHash;
-    IdDependencyHash qmlIdObjectDependenciesPerFunction;
-
-    typedef QHash<QQmlJS::V4IR::Function *, QSet<QQmlPropertyData*> > PropertyDependencyHash;
-    PropertyDependencyHash qmlContextPropertyDependenciesPerFunction;
-    PropertyDependencyHash qmlScopePropertyDependenciesPerFunction;
 };
 
 }
diff --git a/src/qml/compiler/qv4isel_p.cpp b/src/qml/compiler/qv4isel_p.cpp
index 78eb9ba0f5..b86837e167 100644
--- a/src/qml/compiler/qv4isel_p.cpp
+++ b/src/qml/compiler/qv4isel_p.cpp
@@ -148,20 +148,20 @@ void IRDecoder::visitMove(V4IR::Move *s)
         } else if (V4IR::Member *m = s->source->asMember()) {
             if (m->property) {
                 bool captureRequired = true;
-                if (_function && m->attachedPropertiesId == 0) {
-                    if (_function->contextObjectDependencyCandidates.remove(m->property)) {
-                        _function->contextObjectDependencies.insert(m->property);
+
+                Q_ASSERT(m->kind != V4IR::Member::MemberOfEnum);
+                const int attachedPropertiesId = m->attachedPropertiesIdOrEnumValue;
+
+                if (_function && attachedPropertiesId == 0 && !m->property->isConstant()) {
+                    if (m->kind == V4IR::Member::MemberOfQmlContextObject) {
+                        _function->contextObjectPropertyDependencies.insert(m->property->coreIndex, m->property->notifyIndex);
                         captureRequired = false;
-                    } else if (_function->scopeObjectDependencyCandidates.remove(m->property)) {
-                        _function->scopeObjectDependencies.insert(m->property);
+                    } else if (m->kind == V4IR::Member::MemberOfQmlScopeObject) {
+                        _function->scopeObjectPropertyDependencies.insert(m->property->coreIndex, m->property->notifyIndex);
                         captureRequired = false;
                     }
-
-                    if (captureRequired)
-                        captureRequired = !_function->contextObjectDependencies.contains(m->property)
-                                          && !_function->scopeObjectDependencies.contains(m->property);
                 }
-                getQObjectProperty(m->base, m->property->coreIndex, captureRequired, m->attachedPropertiesId, t);
+                getQObjectProperty(m->base, m->property->coreIndex, captureRequired, attachedPropertiesId, t);
                 return;
             } else if (m->base->asTemp() || m->base->asConst()) {
                 getProperty(m->base, *m->name, t);
@@ -200,7 +200,9 @@ void IRDecoder::visitMove(V4IR::Move *s)
     } else if (V4IR::Member *m = s->target->asMember()) {
         if (m->base->asTemp() || m->base->asConst()) {
             if (s->source->asTemp() || s->source->asConst()) {
-                if (m->property && m->attachedPropertiesId == 0) {
+                Q_ASSERT(m->kind != V4IR::Member::MemberOfEnum);
+                const int attachedPropertiesId = m->attachedPropertiesIdOrEnumValue;
+                if (m->property && attachedPropertiesId == 0) {
                     setQObjectProperty(s->source, m->base, m->property->coreIndex);
                     return;
                 } else {
diff --git a/src/qml/compiler/qv4jsir.cpp b/src/qml/compiler/qv4jsir.cpp
index a2dbf44375..deb1af51b4 100644
--- a/src/qml/compiler/qv4jsir.cpp
+++ b/src/qml/compiler/qv4jsir.cpp
@@ -554,8 +554,8 @@ void Subscript::dump(QTextStream &out) const
 
 void Member::dump(QTextStream &out) const
 {
-    if (attachedPropertiesId != 0 && !base->asTemp())
-        out << "[[attached property from " << attachedPropertiesId << "]]";
+    if (kind != MemberOfEnum && attachedPropertiesIdOrEnumValue != 0 && !base->asTemp())
+        out << "[[attached property from " << attachedPropertiesIdOrEnumValue << "]]";
     else
         base->dump(out);
     out << '.' << *name;
@@ -843,17 +843,10 @@ Expr *BasicBlock::SUBSCRIPT(Expr *base, Expr *index)
     return e;
 }
 
-Expr *BasicBlock::MEMBER(Expr *base, const QString *name, QQmlPropertyData *property, int attachedPropertiesId)
+Expr *BasicBlock::MEMBER(Expr *base, const QString *name, QQmlPropertyData *property, uchar kind, int attachedPropertiesIdOrEnumValue)
 {
     Member*e = function->New<Member>();
-    e->init(base, name, property, attachedPropertiesId);
-    return e;
-}
-
-Expr *BasicBlock::MEMBER(Expr *base, const QString *name, int enumValue)
-{
-    Member*e = function->New<Member>();
-    e->init(base, name, enumValue);
+    e->init(base, name, property, kind, attachedPropertiesIdOrEnumValue);
     return e;
 }
 
@@ -1044,10 +1037,7 @@ void CloneExpr::visitSubscript(Subscript *e)
 void CloneExpr::visitMember(Member *e)
 {
     Expr *clonedBase = clone(e->base);
-    if (e->memberIsEnum)
-        cloned = block->MEMBER(clonedBase, e->name, e->enumValue);
-    else
-        cloned = block->MEMBER(clonedBase, e->name, e->property, e->attachedPropertiesId);
+    cloned = block->MEMBER(clonedBase, e->name, e->property, e->kind, e->attachedPropertiesIdOrEnumValue);
 }
 
 } // end of namespace IR
diff --git a/src/qml/compiler/qv4jsir_p.h b/src/qml/compiler/qv4jsir_p.h
index b0ca4c0478..2eba3405fe 100644
--- a/src/qml/compiler/qv4jsir_p.h
+++ b/src/qml/compiler/qv4jsir_p.h
@@ -554,42 +554,49 @@ struct Subscript: Expr {
 };
 
 struct Member: Expr {
+    // Used for property dependency tracking
+    enum MemberKind {
+        UnspecifiedMember,
+        MemberOfEnum,
+        MemberOfQmlScopeObject,
+        MemberOfQmlContextObject
+    };
+
     Expr *base;
     const QString *name;
     QQmlPropertyData *property;
-    int attachedPropertiesId;
-    int enumValue;
-    bool memberIsEnum : 1;
-    bool freeOfSideEffects : 1;
+    int attachedPropertiesIdOrEnumValue; // depending on kind
+    uchar memberIsEnum : 1;
+    uchar freeOfSideEffects : 1;
 
     // This is set for example for for QObject properties. All sorts of extra behavior
     // is defined when writing to them, for example resettable properties are reset
     // when writing undefined to them, and an exception is thrown when they're missing
     // a reset function. And then there's also Qt.binding().
-    bool inhibitTypeConversionOnWrite: 1;
+    uchar inhibitTypeConversionOnWrite: 1;
 
-    void init(Expr *base, const QString *name, QQmlPropertyData *property = 0, int attachedPropertiesId = 0)
+    uchar kind: 3; // MemberKind
+
+    void setEnumValue(int value) {
+        kind = MemberOfEnum;
+        attachedPropertiesIdOrEnumValue = value;
+    }
+
+    void setAttachedPropertiesId(int id) {
+        Q_ASSERT(kind != MemberOfEnum);
+        attachedPropertiesIdOrEnumValue = id;
+    }
+
+    void init(Expr *base, const QString *name, QQmlPropertyData *property = 0, uchar kind = UnspecifiedMember, int attachedPropertiesIdOrEnumValue = 0)
     {
         this->base = base;
         this->name = name;
         this->property = property;
-        this->attachedPropertiesId = attachedPropertiesId;
-        this->enumValue = 0;
+        this->attachedPropertiesIdOrEnumValue = attachedPropertiesIdOrEnumValue;
         this->memberIsEnum = false;
         this->freeOfSideEffects = false;
         this->inhibitTypeConversionOnWrite = property != 0;
-    }
-
-    void init(Expr *base, const QString *name, int enumValue)
-    {
-        this->base = base;
-        this->name = name;
-        this->property = 0;
-        this->attachedPropertiesId = 0;
-        this->enumValue = enumValue;
-        this->memberIsEnum = true;
-        this->freeOfSideEffects = false;
-        this->inhibitTypeConversionOnWrite = false;
+        this->kind = kind;
     }
 
     virtual void accept(ExprVisitor *v) { v->visitMember(this); }
@@ -752,6 +759,9 @@ struct Q_QML_EXPORT Module {
     void setFileName(const QString &name);
 };
 
+// Map from meta property index (existence implies dependency) to notify signal index
+typedef QHash<int, int> PropertyDependencyMap;
+
 struct Function {
     Module *module;
     MemoryPool *pool;
@@ -782,14 +792,8 @@ struct Function {
 
     // Qml extension:
     QSet<int> idObjectDependencies;
-    // Context/Scope properties discovered during identifier resolution
-    QSet<QQmlPropertyData*> contextObjectDependencyCandidates;
-    QSet<QQmlPropertyData*> scopeObjectDependencyCandidates;
-    // Context/Scope properties actually being read from, not only written
-    QSet<QQmlPropertyData*> contextObjectDependencies;
-    QSet<QQmlPropertyData*> scopeObjectDependencies;
-
-    bool hasQmlDependencies() const { return !idObjectDependencies.isEmpty() || !contextObjectDependencies.isEmpty() || !scopeObjectDependencies.isEmpty(); }
+    PropertyDependencyMap contextObjectPropertyDependencies;
+    PropertyDependencyMap scopeObjectPropertyDependencies;
 
     template <typename _Tp> _Tp *New() { return new (pool->allocate(sizeof(_Tp))) _Tp(); }
 
@@ -899,8 +903,7 @@ struct BasicBlock {
     Expr *CALL(Expr *base, ExprList *args = 0);
     Expr *NEW(Expr *base, ExprList *args = 0);
     Expr *SUBSCRIPT(Expr *base, Expr *index);
-    Expr *MEMBER(Expr *base, const QString *name, QQmlPropertyData *property = 0, int attachedPropertiesId = 0);
-    Expr *MEMBER(Expr *base, const QString *name, int enumValue);
+    Expr *MEMBER(Expr *base, const QString *name, QQmlPropertyData *property = 0, uchar kind = Member::UnspecifiedMember, int attachedPropertiesIdOrEnumValue = 0);
 
     Stmt *EXP(Expr *expr);
 
diff --git a/src/qml/compiler/qv4ssa.cpp b/src/qml/compiler/qv4ssa.cpp
index 21c56e3a52..4b6fbc8abe 100644
--- a/src/qml/compiler/qv4ssa.cpp
+++ b/src/qml/compiler/qv4ssa.cpp
@@ -2668,15 +2668,16 @@ void optimizeSSA(Function *function, DefUsesCalculator &defUses)
                     continue;
                 }
                 if (Member *member = m->source->asMember()) {
-                    if (member->memberIsEnum) {
+                    if (member->kind == Member::MemberOfEnum) {
                         Const *c = function->New<Const>();
-                        c->init(SInt32Type, member->enumValue);
+                        const int enumValue = member->attachedPropertiesIdOrEnumValue;
+                        c->init(SInt32Type, enumValue);
                         W += replaceUses(targetTemp, c);
                         defUses.removeDef(*targetTemp);
                         *ref[s] = 0;
                         defUses.removeUse(s, *member->base->asTemp());
                         continue;
-                    } else if (member->attachedPropertiesId != 0 && member->property && member->base->asTemp()) {
+                    } else if (member->attachedPropertiesIdOrEnumValue != 0 && member->property && member->base->asTemp()) {
                         // Attached properties have no dependency on their base. Isel doesn't
                         // need it and we can eliminate the temp used to initialize it.
                         defUses.removeUse(s, *member->base->asTemp());