Make sure we call Codegen::defineFunction with proper arguments

So far, when instantiating QML bindings, the node parameter
could be the same or a child of the body. This will break
badly when we introduce lexical scopeing as that node
could be an AST::Block that opens it's own context.

Changing this requires some smaller adjustments to our autotests,
as error locations will now be slightly different (pointing to
the beginning of the binding, not the beginning of the RHS of
the binding).

Change-Id: I2c536a4fe6d8b549a138cc7967ef034eb2523f3b
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

8 files changed, 45 insertions, 42 deletions

diff --git a/src/qml/compiler/qqmlirbuilder.cpp b/src/qml/compiler/qqmlirbuilder.cpp
index ffad1927df..badf89fe44 100644
--- a/src/qml/compiler/qqmlirbuilder.cpp
+++ b/src/qml/compiler/qqmlirbuilder.cpp
@@ -498,7 +498,7 @@ bool IRBuilder::visit(QQmlJS::AST::UiObjectBinding *node)
 
 bool IRBuilder::visit(QQmlJS::AST::UiScriptBinding *node)
 {
-    appendBinding(node->qualifiedId, node->statement);
+    appendBinding(node->qualifiedId, node->statement, node);
     return false;
 }
 
@@ -958,7 +958,7 @@ bool IRBuilder::visit(QQmlJS::AST::UiPublicMember *node)
                 QQmlJS::AST::Node::accept(node->binding, this);
             } else if (node->statement) {
                 if (!isRedundantNullInitializerForPropertyDeclaration(_propertyDeclaration, node->statement))
-                    appendBinding(node->identifierToken, node->identifierToken, _propertyDeclaration->nameIndex, node->statement);
+                    appendBinding(node->identifierToken, node->identifierToken, _propertyDeclaration->nameIndex, node->statement, node);
             }
             qSwap(_propertyDeclaration, property);
         }
@@ -972,6 +972,7 @@ bool IRBuilder::visit(QQmlJS::AST::UiSourceElement *node)
     if (QQmlJS::AST::FunctionDeclaration *funDecl = QQmlJS::AST::cast<QQmlJS::AST::FunctionDeclaration *>(node->sourceElement)) {
         CompiledFunctionOrExpression *foe = New<CompiledFunctionOrExpression>();
         foe->node = funDecl;
+        foe->parentNode = funDecl;
         foe->nameIndex = registerString(funDecl->name.toString());
         foe->disableAcceleratedLookups = false;
         const int index = _object->functionsAndExpressions->append(foe);
@@ -1045,7 +1046,7 @@ QStringRef IRBuilder::textRefAt(const QQmlJS::AST::SourceLocation &first, const
     return QStringRef(&sourceCode, first.offset, last.offset + last.length - first.offset);
 }
 
-void IRBuilder::setBindingValue(QV4::CompiledData::Binding *binding, QQmlJS::AST::Statement *statement)
+void IRBuilder::setBindingValue(QV4::CompiledData::Binding *binding, QQmlJS::AST::Statement *statement, QQmlJS::AST::Node *parentNode)
 {
     QQmlJS::AST::SourceLocation loc = statement->firstSourceLocation();
     binding->valueLocation.line = loc.startLine;
@@ -1091,6 +1092,7 @@ void IRBuilder::setBindingValue(QV4::CompiledData::Binding *binding, QQmlJS::AST
 
         CompiledFunctionOrExpression *expr = New<CompiledFunctionOrExpression>();
         expr->node = statement;
+        expr->parentNode = parentNode;
         expr->nameIndex = registerString(QLatin1String("expression for ")
                                          + stringAt(binding->propertyNameIndex));
         expr->disableAcceleratedLookups = false;
@@ -1217,7 +1219,7 @@ void IRBuilder::tryGeneratingTranslationBinding(const QStringRef &base, AST::Arg
     }
 }
 
-void IRBuilder::appendBinding(QQmlJS::AST::UiQualifiedId *name, QQmlJS::AST::Statement *value)
+void IRBuilder::appendBinding(QQmlJS::AST::UiQualifiedId *name, QQmlJS::AST::Statement *value, QQmlJS::AST::Node *parentNode)
 {
     const QQmlJS::AST::SourceLocation qualifiedNameLocation = name->identifierToken;
     Object *object = nullptr;
@@ -1228,7 +1230,7 @@ void IRBuilder::appendBinding(QQmlJS::AST::UiQualifiedId *name, QQmlJS::AST::Sta
         return;
     }
     qSwap(_object, object);
-    appendBinding(qualifiedNameLocation, name->identifierToken, registerString(name->name.toString()), value);
+    appendBinding(qualifiedNameLocation, name->identifierToken, registerString(name->name.toString()), value, parentNode);
     qSwap(_object, object);
 }
 
@@ -1243,7 +1245,8 @@ void IRBuilder::appendBinding(QQmlJS::AST::UiQualifiedId *name, int objectIndex,
     qSwap(_object, object);
 }
 
-void IRBuilder::appendBinding(const QQmlJS::AST::SourceLocation &qualifiedNameLocation, const QQmlJS::AST::SourceLocation &nameLocation, quint32 propertyNameIndex, QQmlJS::AST::Statement *value)
+void IRBuilder::appendBinding(const QQmlJS::AST::SourceLocation &qualifiedNameLocation, const QQmlJS::AST::SourceLocation &nameLocation, quint32 propertyNameIndex,
+                              QQmlJS::AST::Statement *value, QQmlJS::AST::Node *parentNode)
 {
     Binding *binding = New<Binding>();
     binding->propertyNameIndex = propertyNameIndex;
@@ -1251,7 +1254,7 @@ void IRBuilder::appendBinding(const QQmlJS::AST::SourceLocation &qualifiedNameLo
     binding->location.line = nameLocation.startLine;
     binding->location.column = nameLocation.startColumn;
     binding->flags = 0;
-    setBindingValue(binding, value);
+    setBindingValue(binding, value, parentNode);
     QString error = bindingsTarget()->appendBinding(binding, /*isListBinding*/false);
     if (!error.isEmpty()) {
         recordError(qualifiedNameLocation, error);
@@ -1813,12 +1816,15 @@ QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(const QList<Compil
     scan.enterGlobalEnvironment(QV4::Compiler::ContextType::Binding);
     for (const CompiledFunctionOrExpression &f : functions) {
         Q_ASSERT(f.node != qmlRoot);
+        Q_ASSERT(f.parentNode && f.parentNode != qmlRoot);
         QQmlJS::AST::FunctionDeclaration *function = QQmlJS::AST::cast<QQmlJS::AST::FunctionDeclaration*>(f.node);
 
-        if (function)
+        if (function) {
             scan.enterQmlFunction(function);
-        else
-            scan.enterEnvironment(f.node, QV4::Compiler::ContextType::Binding);
+        } else {
+            Q_ASSERT(f.node != f.parentNode);
+            scan.enterEnvironment(f.parentNode, QV4::Compiler::ContextType::Binding);
+        }
 
         scan(function ? function->body : f.node);
         scan.leaveEnvironment();
@@ -1863,7 +1869,7 @@ QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(const QList<Compil
         }
 
         _disableAcceleratedLookups = qmlFunction.disableAcceleratedLookups;
-        int idx = defineFunction(name, node,
+        int idx = defineFunction(name, function ? function : qmlFunction.parentNode,
                                  function ? function->formals : nullptr,
                                  body);
         runtimeFunctionIndices[i] = idx;
diff --git a/src/qml/compiler/qqmlirbuilder_p.h b/src/qml/compiler/qqmlirbuilder_p.h
index 1c1ffeb8c9..fa93130252 100644
--- a/src/qml/compiler/qqmlirbuilder_p.h
+++ b/src/qml/compiler/qqmlirbuilder_p.h
@@ -340,12 +340,9 @@ struct Function
 struct Q_QML_PRIVATE_EXPORT CompiledFunctionOrExpression
 {
     CompiledFunctionOrExpression()
-
     {}
-    CompiledFunctionOrExpression(QQmlJS::AST::Node *n)
-        : node(n)
 
-    {}
+    QQmlJS::AST::Node *parentNode = nullptr; // FunctionDeclaration, Statement or Expression
     QQmlJS::AST::Node *node = nullptr; // FunctionDeclaration, Statement or Expression
     quint32 nameIndex = 0;
     bool disableAcceleratedLookups = false;
@@ -516,12 +513,12 @@ public:
     QStringRef textRefAt(const QQmlJS::AST::SourceLocation &first,
                          const QQmlJS::AST::SourceLocation &last) const;
 
-    void setBindingValue(QV4::CompiledData::Binding *binding, QQmlJS::AST::Statement *statement);
+    void setBindingValue(QV4::CompiledData::Binding *binding, QQmlJS::AST::Statement *statement, AST::Node *parentNode);
     void tryGeneratingTranslationBinding(const QStringRef &base, QQmlJS::AST::ArgumentList *args, QV4::CompiledData::Binding *binding);
 
-    void appendBinding(QQmlJS::AST::UiQualifiedId *name, QQmlJS::AST::Statement *value);
+    void appendBinding(QQmlJS::AST::UiQualifiedId *name, QQmlJS::AST::Statement *value, AST::Node *parentNode);
     void appendBinding(QQmlJS::AST::UiQualifiedId *name, int objectIndex, bool isOnAssignment = false);
-    void appendBinding(const QQmlJS::AST::SourceLocation &qualifiedNameLocation, const QQmlJS::AST::SourceLocation &nameLocation, quint32 propertyNameIndex, QQmlJS::AST::Statement *value);
+    void appendBinding(const QQmlJS::AST::SourceLocation &qualifiedNameLocation, const QQmlJS::AST::SourceLocation &nameLocation, quint32 propertyNameIndex, QQmlJS::AST::Statement *value, AST::Node *parentNode);
     void appendBinding(const QQmlJS::AST::SourceLocation &qualifiedNameLocation, const QQmlJS::AST::SourceLocation &nameLocation, quint32 propertyNameIndex, int objectIndex, bool isListItem = false, bool isOnAssignment = false);
 
     bool appendAlias(QQmlJS::AST::UiPublicMember *node);
diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index d0b84c7463..d71cd0cdb3 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -2300,7 +2300,7 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     // at all, because if the onSignal is a signal handler, the user is actually making it explicit
     // that the binding is a function, so we should execute that. However, we don't know that during
     // AOT compilation, so mark the surrounding function as only-returning-a-closure.
-    _context->returnsClosure = cast<ExpressionStatement *>(ast) && cast<FunctionExpression *>(cast<ExpressionStatement *>(ast)->expression);
+    _context->returnsClosure = body && body->statement && cast<ExpressionStatement *>(body->statement) && cast<FunctionExpression *>(cast<ExpressionStatement *>(body->statement)->expression);
 
     BytecodeGenerator bytecode(_context->line, _module->debugMode);
     BytecodeGenerator *savedBytecodeGenerator;
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 52de892a66..5a7ef01eea 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -958,7 +958,7 @@ void tst_qqmlecmascript::deferredPropertiesErrors()
     QVERIFY(!object->objectProperty());
     QVERIFY(!object->objectProperty2());
 
-    QString warning = component.url().toString() + ":6:21: Unable to assign [undefined] to QObject*";
+    QString warning = component.url().toString() + ":6:5: Unable to assign [undefined] to QObject*";
     QTest::ignoreMessage(QtWarningMsg, qPrintable(warning));
 
     qmlExecuteDeferred(object);
@@ -1119,13 +1119,13 @@ void tst_qqmlecmascript::enums()
     {
     QUrl file = testFileUrl("enums.2.qml");
     QString w1 = QLatin1String("QMetaProperty::read: Unable to handle unregistered datatype 'MyEnum' for property 'MyUnregisteredEnumTypeObject::enumProperty'");
-    QString w2 = QLatin1String("QQmlExpression: Expression ") + testFileUrl("enums.2.qml").toString() + QLatin1String(":9:21 depends on non-NOTIFYable properties:");
+    QString w2 = QLatin1String("QQmlExpression: Expression ") + testFileUrl("enums.2.qml").toString() + QLatin1String(":9:5 depends on non-NOTIFYable properties:");
     QString w3 = QLatin1String("    MyUnregisteredEnumTypeObject::enumProperty");
-    QString w4 = file.toString() + ":7:21: Unable to assign [undefined] to int";
-    QString w5 = file.toString() + ":8:21: Unable to assign [undefined] to int";
-    QString w6 = file.toString() + ":9:21: Unable to assign [undefined] to int";
-    QString w7 = file.toString() + ":13:23: Unable to assign [undefined] to [unknown property type]";
-    QString w8 = file.toString() + ":31:23: Unable to assign int to [unknown property type]";
+    QString w4 = file.toString() + ":7:5: Unable to assign [undefined] to int";
+    QString w5 = file.toString() + ":8:5: Unable to assign [undefined] to int";
+    QString w6 = file.toString() + ":9:5: Unable to assign [undefined] to int";
+    QString w7 = file.toString() + ":13:9: Unable to assign [undefined] to [unknown property type]";
+    QString w8 = file.toString() + ":31:9: Unable to assign int to [unknown property type]";
     QTest::ignoreMessage(QtWarningMsg, qPrintable(w1));
     QTest::ignoreMessage(QtWarningMsg, qPrintable(w2));
     QTest::ignoreMessage(QtWarningMsg, qPrintable(w3));
@@ -1319,7 +1319,7 @@ void tst_qqmlecmascript::nonExistentAttachedObject()
 {
     QQmlComponent component(&engine, testFileUrl("nonExistentAttachedObject.qml"));
 
-    QString warning = component.url().toString() + ":4:21: Unable to assign [undefined] to QString";
+    QString warning = component.url().toString() + ":4:5: Unable to assign [undefined] to QString";
     QTest::ignoreMessage(QtWarningMsg, qPrintable(warning));
 
     QObject *object = component.create();
@@ -1935,7 +1935,7 @@ void tst_qqmlecmascript::scriptErrors()
     QString warning3 = url.left(url.length() - 3) + "js:4: Error: Invalid write to global property \"a\"";
     QString warning4 = url + ":13: ReferenceError: a is not defined";
     QString warning5 = url + ":11: ReferenceError: a is not defined";
-    QString warning6 = url + ":10:21: Unable to assign [undefined] to int";
+    QString warning6 = url + ":10:5: Unable to assign [undefined] to int";
     QString warning7 = url + ":15: TypeError: Cannot assign to read-only property \"trueProperty\"";
     QString warning8 = url + ":16: Error: Cannot assign to non-existent property \"fakeProperty\"";
 
@@ -5656,7 +5656,7 @@ void tst_qqmlecmascript::sequenceConversionBindings()
 
     {
         QUrl qmlFile = testFileUrl("sequenceConversion.bindings.error.qml");
-        QString warning = QString(QLatin1String("%1:17:27: Unable to assign QList<int> to QList<bool>")).arg(qmlFile.toString());
+        QString warning = QString(QLatin1String("%1:17:9: Unable to assign QList<int> to QList<bool>")).arg(qmlFile.toString());
         QTest::ignoreMessage(QtWarningMsg, warning.toLatin1().constData());
         QQmlComponent component(&engine, qmlFile);
         QObject *object = component.create();
@@ -6026,10 +6026,10 @@ void tst_qqmlecmascript::functionAssignment_fromBinding()
     QQmlComponent component(&engine, testFileUrl("functionAssignment.1.qml"));
 
     QString url = component.url().toString();
-    QString w1 = url + ":4:25: Unable to assign a function to a property of any type other than var.";
-    QString w2 = url + ":5:25: Invalid use of Qt.binding() in a binding declaration.";
-    QString w3 = url + ":6:21: Invalid use of Qt.binding() in a binding declaration.";
-    QString w4 = url + ":7:15: Invalid use of Qt.binding() in a binding declaration.";
+    QString w1 = url + ":4:5: Unable to assign a function to a property of any type other than var.";
+    QString w2 = url + ":5:5: Invalid use of Qt.binding() in a binding declaration.";
+    QString w3 = url + ":6:5: Invalid use of Qt.binding() in a binding declaration.";
+    QString w4 = url + ":7:5: Invalid use of Qt.binding() in a binding declaration.";
     QTest::ignoreMessage(QtWarningMsg, w1.toLatin1().constData());
     QTest::ignoreMessage(QtWarningMsg, w2.toLatin1().constData());
     QTest::ignoreMessage(QtWarningMsg, w3.toLatin1().constData());
@@ -6881,7 +6881,7 @@ void tst_qqmlecmascript::nonNotifyable()
 
     QString expected1 = QLatin1String("QQmlExpression: Expression ") +
                         component.url().toString() +
-                        QLatin1String(":5:24 depends on non-NOTIFYable properties:");
+                        QLatin1String(":5:5 depends on non-NOTIFYable properties:");
     QString expected2 = QLatin1String("    ") +
                         QLatin1String(object->metaObject()->className()) +
                         QLatin1String("::value");
@@ -7050,7 +7050,7 @@ void tst_qqmlecmascript::switchStatement()
     {
         QQmlComponent component(&engine, testFileUrl("switchStatement.4.qml"));
 
-        QString warning = component.url().toString() + ":4:12: Unable to assign [undefined] to int";
+        QString warning = component.url().toString() + ":4:5: Unable to assign [undefined] to int";
         QTest::ignoreMessage(QtWarningMsg, qPrintable(warning));
 
         MyQmlObject *object = qobject_cast<MyQmlObject *>(component.create());
diff --git a/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp b/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp
index 52e18011cb..9509db9379 100644
--- a/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp
+++ b/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp
@@ -541,7 +541,7 @@ void tst_qqmlengine::outputWarningsToStandardError()
     delete o;
 
     QCOMPARE(messageHandler.messages().count(), 1);
-    QCOMPARE(messageHandler.messages().at(0), QLatin1String("<Unknown File>:1:48: Unable to assign [undefined] to int"));
+    QCOMPARE(messageHandler.messages().at(0), QLatin1String("<Unknown File>:1:32: Unable to assign [undefined] to int"));
     messageHandler.clear();
 
     engine.setOutputWarningsToStandardError(false);
diff --git a/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp b/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp
index 1e9ba80264..b8d5a4d3a3 100644
--- a/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp
+++ b/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp
@@ -2007,15 +2007,15 @@ void tst_qqmlproperty::warnOnInvalidBinding()
     QString expectedWarning;
 
     // V4 error message for property-to-property binding
-    expectedWarning = testUrl.toString() + QString::fromLatin1(":6:36: Unable to assign QQuickText to QQuickRectangle");
+    expectedWarning = testUrl.toString() + QString::fromLatin1(":6:5: Unable to assign QQuickText to QQuickRectangle");
     QTest::ignoreMessage(QtWarningMsg, expectedWarning.toLatin1().constData());
 
     // V8 error message for function-to-property binding
-    expectedWarning = testUrl.toString() + QString::fromLatin1(":7:36: Unable to assign QQuickText to QQuickRectangle");
+    expectedWarning = testUrl.toString() + QString::fromLatin1(":7:5: Unable to assign QQuickText to QQuickRectangle");
     QTest::ignoreMessage(QtWarningMsg, expectedWarning.toLatin1().constData());
 
     // V8 error message for invalid binding to anchor
-    expectedWarning = testUrl.toString() + QString::fromLatin1(":14:33: Unable to assign QQuickItem_QML_8 to QQuickAnchorLine");
+    expectedWarning = testUrl.toString() + QString::fromLatin1(":14:9: Unable to assign QQuickItem_QML_8 to QQuickAnchorLine");
     QTest::ignoreMessage(QtWarningMsg, expectedWarning.toLatin1().constData());
 
     QQmlComponent component(&engine, testUrl);
diff --git a/tests/auto/qml/qqmlvaluetypes/tst_qqmlvaluetypes.cpp b/tests/auto/qml/qqmlvaluetypes/tst_qqmlvaluetypes.cpp
index a456facd2f..1ee4510e30 100644
--- a/tests/auto/qml/qqmlvaluetypes/tst_qqmlvaluetypes.cpp
+++ b/tests/auto/qml/qqmlvaluetypes/tst_qqmlvaluetypes.cpp
@@ -1020,7 +1020,7 @@ void tst_qqmlvaluetypes::bindingAssignment()
 
     // function assignment should fail without crashing
     {
-    QString warning1 = testFileUrl("bindingAssignment.2.qml").toString() + QLatin1String(":6:13: Invalid use of Qt.binding() in a binding declaration.");
+    QString warning1 = testFileUrl("bindingAssignment.2.qml").toString() + QLatin1String(":6:5: Invalid use of Qt.binding() in a binding declaration.");
     QString warning2 = testFileUrl("bindingAssignment.2.qml").toString() + QLatin1String(":10: Cannot assign JavaScript function to value-type property");
     QTest::ignoreMessage(QtWarningMsg, qPrintable(warning1));
     QTest::ignoreMessage(QtWarningMsg, qPrintable(warning2));
@@ -1125,7 +1125,7 @@ void tst_qqmlvaluetypes::autoBindingRemoval()
 
     {
         QQmlComponent component(&engine, testFileUrl("autoBindingRemoval.3.qml"));
-        QString warning = component.url().toString() + ":6:11: Unable to assign [undefined] to QRect";
+        QString warning = component.url().toString() + ":6:5: Unable to assign [undefined] to QRect";
         QTest::ignoreMessage(QtWarningMsg, qPrintable(warning));
         MyTypeObject *object = qobject_cast<MyTypeObject *>(component.create());
         QVERIFY(object != nullptr);
diff --git a/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp b/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp
index 65c9c9e8ad..f2ee8564ef 100644
--- a/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp
+++ b/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp
@@ -5989,7 +5989,7 @@ void tst_qquicktextinput::QTBUG_19956_regexp()
 {
     QUrl url = testFileUrl("qtbug-19956regexp.qml");
 
-    QString warning = url.toString() + ":11:17: Unable to assign [undefined] to QRegExp";
+    QString warning = url.toString() + ":11:9: Unable to assign [undefined] to QRegExp";
     QTest::ignoreMessage(QtWarningMsg, qPrintable(warning));
 
     QQuickView window(url);