Fix broken Maroon game / regression in PropertyChanges {} element

Commit 0aadcf8077840068eb182269e9ed9c31ad12f45e that pre-compiles the
expressions in PropertyChanges {} introduced a regression in where the
evaluation context was incorrect and thus bindings would not be able to
access the correct properties. For example

    PropertyChanges {
        target: someObject
        y: height / 2
    }

Here height should be looked up in the context of "someObject", not of the
PropertyChanges element.

This patch introduces an auto-test that verifies that the lookup context is
correct and fixes the bug by disabling accelerated compile time property
lookups for binding expressions that are requested from a custom parser.

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

9 files changed, 167 insertions, 23 deletions

diff --git a/src/qml/compiler/qqmlcodegenerator.cpp b/src/qml/compiler/qqmlcodegenerator.cpp
index 1b96bad94f..8809221abe 100644
--- a/src/qml/compiler/qqmlcodegenerator.cpp
+++ b/src/qml/compiler/qqmlcodegenerator.cpp
@@ -281,7 +281,7 @@ bool QQmlCodeGenerator::sanityCheckFunctionNames()
 {
     QSet<QString> functionNames;
     for (Function *f = _object->functions->first; f; f = f->next) {
-        AST::FunctionDeclaration *function = AST::cast<AST::FunctionDeclaration*>(_functions.at(f->index));
+        AST::FunctionDeclaration *function = AST::cast<AST::FunctionDeclaration*>(_functions.at(f->index).node);
         Q_ASSERT(function);
         QString name = function->name.toString();
         if (functionNames.contains(name))
@@ -1210,6 +1210,7 @@ JSCodeGen::JSCodeGen(const QString &fileName, const QString &sourceCode, V4IR::M
     , jsEngine(jsEngine)
     , qmlRoot(qmlRoot)
     , imports(imports)
+    , _disableAcceleratedLookups(false)
     , _contextObject(0)
     , _scopeObject(0)
     , _contextObjectTemp(-1)
@@ -1234,23 +1235,23 @@ void JSCodeGen::beginObjectScope(QQmlPropertyCache *scopeObject)
     _scopeObject = scopeObject;
 }
 
-QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(const QList<AST::Node*> &functions, const QHash<int, QString> &functionNames)
+QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(const QList<CompiledFunctionOrExpression> &functions)
 {
     QVector<int> runtimeFunctionIndices(functions.size());
 
     ScanFunctions scan(this, sourceCode, GlobalCode);
     scan.enterEnvironment(0, QmlBinding);
     scan.enterQmlScope(qmlRoot, QStringLiteral("context scope"));
-    foreach (AST::Node *node, functions) {
-        Q_ASSERT(node != qmlRoot);
-        AST::FunctionDeclaration *function = AST::cast<AST::FunctionDeclaration*>(node);
+    foreach (const CompiledFunctionOrExpression &f, functions) {
+        Q_ASSERT(f.node != qmlRoot);
+        AST::FunctionDeclaration *function = AST::cast<AST::FunctionDeclaration*>(f.node);
 
         if (function)
             scan.enterQmlFunction(function);
         else
-            scan.enterEnvironment(node, QmlBinding);
+            scan.enterEnvironment(f.node, QmlBinding);
 
-        scan(function ? function->body : node);
+        scan(function ? function->body : f.node);
         scan.leaveEnvironment();
     }
     scan.leaveEnvironment();
@@ -1260,7 +1261,8 @@ QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(const QList<AST::N
     _function = _module->functions.at(defineFunction(QStringLiteral("context scope"), qmlRoot, 0, 0));
 
     for (int i = 0; i < functions.count(); ++i) {
-        AST::Node *node = functions.at(i);
+        const CompiledFunctionOrExpression &qmlFunction = functions.at(i);
+        AST::Node *node = qmlFunction.node;
         Q_ASSERT(node != qmlRoot);
 
         AST::FunctionDeclaration *function = AST::cast<AST::FunctionDeclaration*>(node);
@@ -1268,8 +1270,10 @@ QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(const QList<AST::N
         QString name;
         if (function)
             name = function->name.toString();
+        else if (!qmlFunction.name.isEmpty())
+            name = qmlFunction.name;
         else
-            name = functionNames.value(i, QStringLiteral("%qml-expression-entry"));
+            name = QStringLiteral("%qml-expression-entry");
 
         AST::SourceElements *body;
         if (function)
@@ -1289,6 +1293,7 @@ QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(const QList<AST::N
             body = body->finish();
         }
 
+        _disableAcceleratedLookups = qmlFunction.disableAcceleratedLookups;
         int idx = defineFunction(name, node,
                                  function ? function->formals : 0,
                                  body);
@@ -1530,6 +1535,9 @@ void JSCodeGen::beginFunctionBodyHook()
 
 V4IR::Expr *JSCodeGen::fallbackNameLookup(const QString &name, int line, int col)
 {
+    if (_disableAcceleratedLookups)
+        return 0;
+
     Q_UNUSED(line)
     Q_UNUSED(col)
     // Implement QML lookup semantics in the current file context.
@@ -1746,7 +1754,7 @@ bool SignalHandlerConverter::convertSignalHandlerExpressionsToFunctionDeclaratio
         if (paramList)
             paramList = paramList->finish();
 
-        AST::Statement *statement = static_cast<AST::Statement*>(parsedQML->functions[binding->value.compiledScriptIndex]);
+        AST::Statement *statement = static_cast<AST::Statement*>(parsedQML->functions[binding->value.compiledScriptIndex].node);
         AST::SourceElement *sourceElement = new (pool) AST::StatementSourceElement(statement);
         AST::SourceElements *elements = new (pool) AST::SourceElements(sourceElement);
         elements = elements->finish();
diff --git a/src/qml/compiler/qqmlcodegenerator_p.h b/src/qml/compiler/qqmlcodegenerator_p.h
index 348ef0a2cf..0a0e4f2d5b 100644
--- a/src/qml/compiler/qqmlcodegenerator_p.h
+++ b/src/qml/compiler/qqmlcodegenerator_p.h
@@ -166,6 +166,20 @@ struct Pragma
     QV4::CompiledData::Location location;
 };
 
+struct CompiledFunctionOrExpression
+{
+    CompiledFunctionOrExpression()
+        : disableAcceleratedLookups(false)
+    {}
+    CompiledFunctionOrExpression(AST::Node *n)
+        : node(n)
+        , disableAcceleratedLookups(false)
+    {}
+    AST::Node *node; // FunctionDeclaration, Statement or Expression
+    QString name;
+    bool disableAcceleratedLookups;
+};
+
 struct ParsedQML
 {
     ParsedQML(bool debugMode)
@@ -180,7 +194,7 @@ struct ParsedQML
     AST::UiProgram *program;
     int indexOfRootObject;
     QList<QmlObject*> objects;
-    QList<AST::Node*> functions; // FunctionDeclaration, Statement or Expression
+    QList<CompiledFunctionOrExpression> functions;
     QV4::Compiler::JSUnitGenerator jsGenerator;
 
     QV4::CompiledData::TypeReferenceMap typeReferences;
@@ -269,7 +283,7 @@ public:
     QList<QV4::CompiledData::Import*> _imports;
     QList<Pragma*> _pragmas;
     QList<QmlObject*> _objects;
-    QList<AST::Node*> _functions;
+    QList<CompiledFunctionOrExpression> _functions;
 
     QV4::CompiledData::TypeReferenceMap _typeReferences;
 
@@ -362,7 +376,7 @@ struct Q_QML_EXPORT JSCodeGen : public QQmlJS::Codegen
     void beginObjectScope(QQmlPropertyCache *scopeObject);
 
     // Returns mapping from input functions to index in V4IR::Module::functions / compiledData->runtimeFunctions
-    QVector<int> generateJSCodeForFunctionsAndBindings(const QList<AST::Node*> &functions, const QHash<int, QString> &functionNames);
+    QVector<int> generateJSCodeForFunctionsAndBindings(const QList<CompiledFunctionOrExpression> &functions);
 
 protected:
     virtual void beginFunctionBodyHook();
@@ -376,6 +390,7 @@ private:
     AST::UiProgram *qmlRoot;
     QQmlTypeNameCache *imports;
 
+    bool _disableAcceleratedLookups;
     ObjectIdMapping _idObjects;
     QQmlPropertyCache *_contextObject;
     QQmlPropertyCache *_scopeObject;
diff --git a/src/qml/qml/qqmlcompiler.cpp b/src/qml/qml/qqmlcompiler.cpp
index 2b925b24ec..1e5f6bfa34 100644
--- a/src/qml/qml/qqmlcompiler.cpp
+++ b/src/qml/qml/qqmlcompiler.cpp
@@ -62,7 +62,6 @@
 #include "qqmlglobal_p.h"
 #include "qqmlbinding_p.h"
 #include "qqmlabstracturlinterceptor_p.h"
-#include "qqmlcodegenerator_p.h"
 
 #include <QDebug>
 #include <QPointF>
@@ -2707,6 +2706,10 @@ int QQmlCompiler::bindingIdentifier(const QString &name, const Variant &value, c
     reference->value = 0;
     reference->bindingContext = ctxt;
     reference->bindingContext.owner++;
+    // Unfortunately this is required for example for PropertyChanges where the bindings
+    // will be executed in the dynamic scope of the target, so we can't resolve any lookups
+    // at run-time.
+    reference->disableLookupAcceleration = true;
 
     const int id = output->customParserBindings.count();
     output->customParserBindings.append(0); // Filled in later.
@@ -3684,9 +3687,12 @@ bool QQmlCompiler::completeComponentBuild()
         }
 
         ComponentCompileState::PerObjectCompileData *cd = &compileState->jsCompileData[b->bindingContext.object];
-        cd->functionsToCompile.append(node);
+        QtQml::CompiledFunctionOrExpression f;
+        f.node = node;
+        f.name = binding.property->name().toString().prepend(QStringLiteral("expression for "));
+        f.disableAcceleratedLookups = binding.disableLookupAcceleration;
+        cd->functionsToCompile.append(f);
         binding.compiledIndex = cd->functionsToCompile.count() - 1;
-        cd->expressionNames.insert(binding.compiledIndex, binding.property->name().toString().prepend(QStringLiteral("expression for ")));
 
         if (componentStats && b->value)
             componentStats->componentStat.scriptBindings.append(b->value->location);
@@ -3719,7 +3725,7 @@ bool QQmlCompiler::completeComponentBuild()
 
             jsCodeGen.beginObjectScope(scopeObject->metatype);
 
-            cd->runtimeFunctionIndices = jsCodeGen.generateJSCodeForFunctionsAndBindings(cd->functionsToCompile, cd->expressionNames);
+            cd->runtimeFunctionIndices = jsCodeGen.generateJSCodeForFunctionsAndBindings(cd->functionsToCompile);
             QList<QQmlError> errors = jsCodeGen.errors();
             if (!errors.isEmpty()) {
                 exceptions << errors;
diff --git a/src/qml/qml/qqmlcompiler_p.h b/src/qml/qml/qqmlcompiler_p.h
index 75f404a7d5..516f6653ca 100644
--- a/src/qml/qml/qqmlcompiler_p.h
+++ b/src/qml/qml/qqmlcompiler_p.h
@@ -62,6 +62,7 @@
 #include "qqmlpropertycache_p.h"
 #include "qqmltypenamecache_p.h"
 #include "qqmltypeloader_p.h"
+#include <private/qqmlcodegenerator_p.h>
 #include "private/qv4identifier_p.h"
 #include <private/qqmljsastfwd_p.h>
 
@@ -231,14 +232,15 @@ namespace QQmlCompilerTypes {
     struct JSBindingReference : public QQmlPool::Class,
                                 public BindingReference
     {
-        JSBindingReference() : nextReference(0) {}
+        JSBindingReference() : disableLookupAcceleration(false), nextReference(0) {}
 
         QQmlScript::Variant expression;
         QQmlScript::Property *property;
         QQmlScript::Value *value;
 
         int compiledIndex : 16;
-        int customParserBindingsIndex : 16;
+        int customParserBindingsIndex : 15;
+        int disableLookupAcceleration: 1;
 
         BindingContext bindingContext;
 
@@ -319,10 +321,9 @@ namespace QQmlCompilerTypes {
         QList<CompiledMetaMethod> compiledMetaMethods;
         struct PerObjectCompileData
         {
-            QList<QQmlJS::AST::Node*> functionsToCompile;
+            QList<QtQml::CompiledFunctionOrExpression> functionsToCompile;
             QVector<int> runtimeFunctionIndices;
             QVector<CompiledMetaMethod> compiledMetaMethods;
-            QHash<int, QString> expressionNames;
         };
         QHash<QQmlScript::Object *, PerObjectCompileData> jsCompileData;
     };
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index 7d377af0f8..c9694da0df 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -2372,8 +2372,7 @@ void QQmlTypeData::compile()
         // Compile JS binding expressions and signal handlers
 
         JSCodeGen jsCodeGen(finalUrlString(), parsedQML->code, &parsedQML->jsModule, &parsedQML->jsParserEngine, parsedQML->program, m_compiledData->importCache);
-        QHash<int, QString> expressionNames; // ### TODO
-        const QVector<int> runtimeFunctionIndices = jsCodeGen.generateJSCodeForFunctionsAndBindings(parsedQML->functions, expressionNames);
+        const QVector<int> runtimeFunctionIndices = jsCodeGen.generateJSCodeForFunctionsAndBindings(parsedQML->functions);
 
         QV4::ExecutionEngine *v4 = QV8Engine::getV4(m_typeLoader->engine());
 
diff --git a/tests/auto/qml/qqmllanguage/data/customParserBindingScopes.qml b/tests/auto/qml/qqmllanguage/data/customParserBindingScopes.qml
new file mode 100644
index 0000000000..2efc199f32
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/customParserBindingScopes.qml
@@ -0,0 +1,19 @@
+import Test 1.0
+import QtQml 2.0
+QtObject {
+    id: root
+
+    property int otherProperty: 10
+
+    property QtObject child: QtObject {
+        id: child
+
+        property int testProperty;
+        property int otherProperty: 41
+
+        property QtObject customBinder: CustomBinding {
+            target: child
+            testProperty: otherProperty + 1
+        }
+    }
+}
diff --git a/tests/auto/qml/qqmllanguage/testtypes.cpp b/tests/auto/qml/qqmllanguage/testtypes.cpp
index 3957f9d872..4a4ab3b81a 100644
--- a/tests/auto/qml/qqmllanguage/testtypes.cpp
+++ b/tests/auto/qml/qqmllanguage/testtypes.cpp
@@ -89,6 +89,8 @@ void registerTypes()
 
     qmlRegisterUncreatableType<MyUncreateableBaseClass,1>("Test", 1, 1, "MyUncreateableBaseClass", "Cannot create MyUncreateableBaseClass");
     qmlRegisterType<MyCreateableDerivedClass,1>("Test", 1, 1, "MyCreateableDerivedClass");
+
+    qmlRegisterCustomType<CustomBinding>("Test", 1, 0, "CustomBinding", new CustomBindingParser);
 }
 
 QVariant myCustomVariantTypeConverter(const QString &data)
@@ -97,3 +99,61 @@ QVariant myCustomVariantTypeConverter(const QString &data)
     rv.a = data.toInt();
     return QVariant::fromValue(rv);
 }
+
+
+QByteArray CustomBindingParser::compile(const QList<QQmlCustomParserProperty> &properties)
+{
+    QByteArray result;
+    QDataStream ds(&result, QIODevice::WriteOnly);
+
+    ds << properties.count();
+    for (int i = 0; i < properties.count(); ++i) {
+        const QQmlCustomParserProperty &prop = properties.at(i);
+        ds << prop.name();
+
+        Q_ASSERT(prop.assignedValues().count() == 1);
+        QVariant value = prop.assignedValues().first();
+
+        Q_ASSERT(value.userType() == qMetaTypeId<QQmlScript::Variant>());
+        QQmlScript::Variant v = qvariant_cast<QQmlScript::Variant>(value);
+        Q_ASSERT(v.type() == QQmlScript::Variant::Script);
+        int bindingId = bindingIdentifier(v, prop.name());
+        ds << bindingId;
+
+        ds << prop.location().line;
+    }
+
+    return result;
+}
+
+void CustomBindingParser::setCustomData(QObject *object, const QByteArray &data)
+{
+    CustomBinding *customBinding = qobject_cast<CustomBinding*>(object);
+    Q_ASSERT(customBinding);
+    customBinding->m_bindingData = data;
+}
+
+void CustomBinding::componentComplete()
+{
+    Q_ASSERT(m_target);
+
+    QDataStream ds(m_bindingData);
+    int count;
+    ds >> count;
+    for (int i = 0; i < count; ++i) {
+        QString name;
+        ds >> name;
+
+        int bindingId;
+        ds >> bindingId;
+
+        int line;
+        ds >> line;
+
+        QQmlBinding *binding = QQmlBinding::createBinding(QQmlBinding::Identifier(bindingId), m_target, qmlContext(this), QString(), line);
+
+        QQmlProperty property(m_target, name, qmlContext(this));
+        binding->setTarget(property);
+        QQmlPropertyPrivate::setBinding(property, binding);
+    }
+}
diff --git a/tests/auto/qml/qqmllanguage/testtypes.h b/tests/auto/qml/qqmllanguage/testtypes.h
index 703b26a73c..a968d9a25a 100644
--- a/tests/auto/qml/qqmllanguage/testtypes.h
+++ b/tests/auto/qml/qqmllanguage/testtypes.h
@@ -1070,6 +1070,29 @@ QML_DECLARE_TYPE(MyRevisionedSubclass)
 QML_DECLARE_TYPE(MySubclass)
 QML_DECLARE_TYPE(MyReceiversTestObject)
 
+class CustomBinding : public QObject, public QQmlParserStatus
+{
+    Q_OBJECT
+    Q_INTERFACES(QQmlParserStatus)
+    Q_PROPERTY(QObject* target READ target WRITE setTarget)
+public:
+
+    virtual void classBegin() {}
+    virtual void componentComplete();
+
+    QObject *target() const { return m_target; }
+    void setTarget(QObject *newTarget) { m_target = newTarget; }
+
+    QPointer<QObject> m_target;
+    QByteArray m_bindingData;
+};
+
+class CustomBindingParser : public QQmlCustomParser
+{
+    virtual QByteArray compile(const QList<QQmlCustomParserProperty> &properties);
+    virtual void setCustomData(QObject *object, const QByteArray &data);
+};
+
 void registerTypes();
 
 #endif // TESTTYPES_H
diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 621061ab6a..6a577ec91c 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -214,6 +214,8 @@ private slots:
     void compositeSingletonSelectors();
     void compositeSingletonRegistered();
 
+    void customParserBindingScopes();
+
 private:
     QQmlEngine engine;
     QStringList defaultImportPathList;
@@ -3534,6 +3536,17 @@ void tst_qqmllanguage::compositeSingletonRegistered()
     verifyCompositeSingletonPropertyValues(o, "value1", 925, "value2", 755);
 }
 
+void tst_qqmllanguage::customParserBindingScopes()
+{
+    QQmlComponent component(&engine, testFile("customParserBindingScopes.qml"));
+    VERIFY_ERRORS(0);
+    QScopedPointer<QObject> o(component.create());
+    QVERIFY(!o.isNull());
+    QPointer<QObject> child = qvariant_cast<QObject*>(o->property("child"));
+    QVERIFY(!child.isNull());
+    QCOMPARE(child->property("testProperty").toInt(), 42);
+}
+
 QTEST_MAIN(tst_qqmllanguage)
 
 #include "tst_qqmllanguage.moc"