From 99178a947c0c73ce400d2dce1c7da2c1b624a634 Mon Sep 17 00:00:00 2001 From: Andrei Golubev Date: Wed, 1 Jun 2022 16:47:37 +0200 Subject: QmlCompiler: Fix script indices calculation The logic is misbehaving on multiple occasions. For instance, same-named signal handlers in different scopes and script bindings inside array scopes were wrongly handled. Fix that by revising the mechanism of inner function computation As a drive by, fix parseLiteralOrScriptBinding() to distinguish translation bindings from script bindings (they are vitally different now) Extend the script calculation test in tst_qqmljsscope to cover the findings Pick-to: 6.4 Change-Id: Ic4cf0a4539f0d714a416b61f4635eb6494e89922 Reviewed-by: Maximilian Goldstein --- .../qqmljsscope/QQmlJSScopeTests/CMakeLists.txt | 1 + .../QQmlJSScopeTests/typewithproperties.h | 56 +++++++++++++++ .../qqmljsscope/data/functionAndBindingIndices.qml | 37 ++++++++++ tests/auto/qml/qqmljsscope/tst_qqmljsscope.cpp | 80 +++++++++++++++++----- 4 files changed, 156 insertions(+), 18 deletions(-) create mode 100644 tests/auto/qml/qqmljsscope/QQmlJSScopeTests/typewithproperties.h (limited to 'tests/auto/qml/qqmljsscope') diff --git a/tests/auto/qml/qqmljsscope/QQmlJSScopeTests/CMakeLists.txt b/tests/auto/qml/qqmljsscope/QQmlJSScopeTests/CMakeLists.txt index cd51b57cbe..83d509c45a 100644 --- a/tests/auto/qml/qqmljsscope/QQmlJSScopeTests/CMakeLists.txt +++ b/tests/auto/qml/qqmljsscope/QQmlJSScopeTests/CMakeLists.txt @@ -11,6 +11,7 @@ qt6_add_qml_module(qqmljsscope_test_module SOURCES singleton.h singleton.cpp extensiontypes.h + typewithproperties.h ) qt_autogen_tools_initial_setup(qqmljsscope_test_moduleplugin) diff --git a/tests/auto/qml/qqmljsscope/QQmlJSScopeTests/typewithproperties.h b/tests/auto/qml/qqmljsscope/QQmlJSScopeTests/typewithproperties.h new file mode 100644 index 0000000000..c8107f1ea4 --- /dev/null +++ b/tests/auto/qml/qqmljsscope/QQmlJSScopeTests/typewithproperties.h @@ -0,0 +1,56 @@ +/**************************************************************************** +** +** Copyright (C) 2022 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:GPL-EXCEPT$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#ifndef TYPEWITHPROPERTIES_H +#define TYPEWITHPROPERTIES_H + +#include +#include +#include +#include +#include + +class TypeWithProperties : public QObject +{ + Q_OBJECT + QML_ELEMENT + + Q_PROPERTY(double a READ a WRITE setA NOTIFY aChanged) + + QProperty m_a { 0.0 }; + +public: + TypeWithProperties(QObject *parent = nullptr) : QObject(parent) { } + double a() const { return m_a; } + void setA(double a_) { m_a = a_; } + +Q_SIGNALS: + void aChanged(); +}; + +#endif // TYPEWITHPROPERTIES_H diff --git a/tests/auto/qml/qqmljsscope/data/functionAndBindingIndices.qml b/tests/auto/qml/qqmljsscope/data/functionAndBindingIndices.qml index 3108cc1568..d6f5d39d76 100644 --- a/tests/auto/qml/qqmljsscope/data/functionAndBindingIndices.qml +++ b/tests/auto/qml/qqmljsscope/data/functionAndBindingIndices.qml @@ -1,5 +1,6 @@ import QtQml import QtQuick +import QQmlJSScopeTests 1.0 Text { id: root @@ -102,6 +103,33 @@ Text { } } + property list itemList: [ + Text { + function jsInsideArrayScope() { return 42; } + }, + Item { + property var x123: function(a) { return a + 77; } + function jsInsideArrayScope2(flag) { + var closure = () => { return "foobar"; }; + if (flag) + return closure; + return () => { return "bazbar"; }; + } + }, + Rectangle {} // dummy + ] + + // translations: + property string translated1: qsTr("bad but ok") + property string translated2: qsTrId("bad_but_ok_id") + property string translated3: qsTr("bad but ok") + "?" + property string translated4: qsTrId("bad_but_ok_id") + "?" + + property string translated5 + property string translated6 + translated5: qsTr("bad but ok") + translated6: qsTrId("bad_but_ok_id") + // function with nested one function jsFunctionReturningFunctions() { return [ function() { return 42 }, () => { return "42" } ]; @@ -111,4 +139,13 @@ Text { function bindText() { root.text = Qt.binding( function() { return jsFuncTyped("foo") + "bar"; } ); } + + // special code which fails somehow: + TypeWithProperties { + onAChanged: console.log("x"); + } + + TypeWithProperties { + onAChanged: function() { } + } } diff --git a/tests/auto/qml/qqmljsscope/tst_qqmljsscope.cpp b/tests/auto/qml/qqmljsscope/tst_qqmljsscope.cpp index 0686fd22ac..ff72534a3e 100644 --- a/tests/auto/qml/qqmljsscope/tst_qqmljsscope.cpp +++ b/tests/auto/qml/qqmljsscope/tst_qqmljsscope.cpp @@ -457,6 +457,56 @@ inline QString getScopeName(const QQmlJSScope::ConstPtr &scope) return scope->baseTypeName(); } +struct FunctionOrExpressionIdentifier +{ + QString name; + QQmlJS::SourceLocation loc = QQmlJS::SourceLocation {}; // source location of owning scope + int index = -1; // relative or absolute script index + FunctionOrExpressionIdentifier() = default; + FunctionOrExpressionIdentifier(const QString &name, const QQmlJS::SourceLocation &l, int i) + : name(name), loc(l), index(i) + { + } + FunctionOrExpressionIdentifier(const QString &name, const QV4::CompiledData::Location &l, int i) + : name(name), loc(QQmlJS::SourceLocation { 0, 0, l.line(), l.column() }), index(i) + { + } + + friend bool operator<(const FunctionOrExpressionIdentifier &x, + const FunctionOrExpressionIdentifier &y) + { + // source location is taken from the owner scope so would be non-unique, + // while name must be unique within that scope. so: if source locations + // match, compare by name + if (x.loc == y.loc) + return x.name < y.name; + + // otherwise, compare by start line first, if they match, then by column + if (x.loc.startLine == y.loc.startLine) + return x.loc.startColumn < y.loc.startColumn; + return x.loc.startLine < y.loc.startLine; + } + friend bool operator==(const FunctionOrExpressionIdentifier &x, + const FunctionOrExpressionIdentifier &y) + { + // equal-compare by name and index + return x.index == y.index && x.name == y.name; + } + friend bool operator!=(const FunctionOrExpressionIdentifier &x, + const FunctionOrExpressionIdentifier &y) + { + return !(x == y); + } + friend QDebug &operator<<(QDebug &stream, const FunctionOrExpressionIdentifier &x) + { + const QString dump = u"(%1, %2, loc %3:%4)"_s.arg(x.name, QString::number(x.index), + QString::number(x.loc.startLine), + QString::number(x.loc.startColumn)); + stream << dump; + return stream.maybeSpace(); + } +}; + void tst_qqmljsscope::scriptIndices() { { @@ -473,31 +523,30 @@ void tst_qqmljsscope::scriptIndices() QVERIFY(root); QVERIFY(document.javaScriptCompilationUnit.unitData()); - using IndexedString = std::pair; // compare QQmlJSScope and QmlIR: // {property, function}Name and relative (per-object) function table index - QList orderedJSScopeExpressionsRelative; - QList orderedQmlIrExpressionsRelative; + QList orderedJSScopeExpressionsRelative; + QList orderedQmlIrExpressionsRelative; // {property, function}Name and absolute (per-document) function table index - QList orderedJSScopeExpressionsAbsolute; - QList orderedQmlIrExpressionsAbsolute; + QList orderedJSScopeExpressionsAbsolute; + QList orderedQmlIrExpressionsAbsolute; const auto populateQQmlJSScopeArrays = [&](const QQmlJSScope::ConstPtr &scope, const QString &name, QQmlJSMetaMethod::RelativeFunctionIndex relativeIndex) { - orderedJSScopeExpressionsRelative.emplaceBack(name, + orderedJSScopeExpressionsRelative.emplaceBack(name, scope->sourceLocation(), static_cast(relativeIndex)); auto absoluteIndex = scope->ownRuntimeFunctionIndex(relativeIndex); - orderedJSScopeExpressionsAbsolute.emplaceBack(name, + orderedJSScopeExpressionsAbsolute.emplaceBack(name, scope->sourceLocation(), static_cast(absoluteIndex)); }; const auto populateQmlIRArrays = [&](const QmlIR::Object *irObject, const QString &name, int relative) { - orderedQmlIrExpressionsRelative.emplaceBack(name, relative); + orderedQmlIrExpressionsRelative.emplaceBack(name, irObject->location, relative); auto absolute = irObject->runtimeFunctionIndices.at(relative); - orderedQmlIrExpressionsAbsolute.emplaceBack(name, absolute); + orderedQmlIrExpressionsAbsolute.emplaceBack(name, irObject->location, absolute); }; const auto suitableScope = [](const QQmlJSScope::ConstPtr &scope) { @@ -513,7 +562,6 @@ void tst_qqmljsscope::scriptIndices() queue.pop_front(); if (suitableScope(current)) { - const auto methods = current->ownMethods(); for (const auto &method : methods) { if (method.methodType() == QQmlJSMetaMethod::Signal) @@ -561,15 +609,11 @@ void tst_qqmljsscope::scriptIndices() } } - auto less = [](const IndexedString &x, const IndexedString &y) { return x.first < y.first; }; - std::sort(orderedJSScopeExpressionsRelative.begin(), orderedJSScopeExpressionsRelative.end(), - less); - std::sort(orderedQmlIrExpressionsRelative.begin(), orderedQmlIrExpressionsRelative.end(), less); - - std::sort(orderedJSScopeExpressionsAbsolute.begin(), orderedJSScopeExpressionsAbsolute.end(), - less); - std::sort(orderedQmlIrExpressionsAbsolute.begin(), orderedQmlIrExpressionsAbsolute.end(), less); + std::sort(orderedJSScopeExpressionsRelative.begin(), orderedJSScopeExpressionsRelative.end()); + std::sort(orderedQmlIrExpressionsRelative.begin(), orderedQmlIrExpressionsRelative.end()); + std::sort(orderedJSScopeExpressionsAbsolute.begin(), orderedJSScopeExpressionsAbsolute.end()); + std::sort(orderedQmlIrExpressionsAbsolute.begin(), orderedQmlIrExpressionsAbsolute.end()); QCOMPARE(orderedJSScopeExpressionsRelative, orderedQmlIrExpressionsRelative); QCOMPARE(orderedJSScopeExpressionsAbsolute, orderedQmlIrExpressionsAbsolute); } -- cgit v1.2.3