From 15d23885128d00b49f78fc43c4cc1080cc0f4520 Mon Sep 17 00:00:00 2001 From: Sami Shalayel Date: Tue, 11 Oct 2022 11:41:13 +0200 Subject: qmltc: Remove broken alias optimization Remove the recursive alias resolution mechanism. It tried to resolve aliases recursively but only knew the id's of the current component. This means that an alias pointing to a property defined in another component (e.g. a different qml file) was resolved using the id-to-scope map of the component containing the alias property. This leads to unresolved aliases at best and endless loops when the aliased property was itself an alias pointing to some property also existing in the original component (see QmltcTests/ComponentWithAlias{1..3}.qml for an example). Also added the reproducer in the qmltc tests. Fixes: QTBUG-107533 Fixes: QTBUG-107534 Change-Id: Idbf0cff3e45213bc7fbb4a98a9393c6754b00046 Reviewed-by: Fabian Kosmale (cherry picked from commit 259ff52e64cd8725bfaa7330a33f8d36456e8b1a) --- src/qmlcompiler/qqmljsutils.cpp | 18 +++++++----- tests/auto/qml/qmltc/QmltcTests/CMakeLists.txt | 4 +++ .../qml/qmltc/QmltcTests/ComponentWithAlias1.qml | 8 ++++++ .../qml/qmltc/QmltcTests/ComponentWithAlias2.qml | 8 ++++++ .../qml/qmltc/QmltcTests/ComponentWithAlias3.qml | 5 ++++ tests/auto/qml/qmltc/QmltcTests/aliases.qml | 29 ++++++++++++++++++++ tests/auto/qml/qmltc/tst_qmltc.cpp | 32 +++++++++++++++++++++- tests/auto/qml/qmltc/tst_qmltc.h | 1 + 8 files changed, 97 insertions(+), 8 deletions(-) create mode 100644 tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias1.qml create mode 100644 tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias2.qml create mode 100644 tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias3.qml create mode 100644 tests/auto/qml/qmltc/QmltcTests/aliases.qml diff --git a/src/qmlcompiler/qqmljsutils.cpp b/src/qmlcompiler/qqmljsutils.cpp index c07c0b6845..b4c8dfde06 100644 --- a/src/qmlcompiler/qqmljsutils.cpp +++ b/src/qmlcompiler/qqmljsutils.cpp @@ -25,17 +25,23 @@ resolveAlias(ScopeForId scopeForId, const QQmlJSMetaProperty &property, QQmlJSUtils::ResolvedAlias result {}; result.owner = owner; - for (QQmlJSMetaProperty nextProperty = property; nextProperty.isAlias();) { - - // this is a special (seemingly useless) section which is necessary when - // we have an alias pointing to an alias. this way we avoid a check - // whether a property is an alias at the very end of the loop body + // TODO: one could optimize the generated alias code for aliases pointing to aliases + // e.g., if idA.myAlias -> idB.myAlias2 -> idC.myProp, then one could directly generate + // idA.myProp as pointing to idC.myProp. + // This gets complicated when idB.myAlias is in a different Component than where the + // idA.myAlias is defined: scopeForId currently only contains the ids of the current + // component and alias resolution on the ids of a different component fails then. + if (QQmlJSMetaProperty nextProperty = property; nextProperty.isAlias()) { QQmlJSScope::ConstPtr resultOwner = result.owner; result = QQmlJSUtils::ResolvedAlias {}; visitor.reset(); auto aliasExprBits = nextProperty.aliasExpression().split(u'.'); + // do not crash on invalid aliasexprbits when accessing aliasExprBits[0] + if (aliasExprBits.size() < 1) + return {}; + // resolve id first: resultOwner = scopeForId(aliasExprBits[0], resultOwner); if (!resultOwner) @@ -46,8 +52,6 @@ resolveAlias(ScopeForId scopeForId, const QQmlJSMetaProperty &property, aliasExprBits.removeFirst(); // Note: for simplicity, remove the result.owner = resultOwner; result.kind = QQmlJSUtils::AliasTarget_Object; - // reset the property to avoid endless loop when aliasExprBits is empty - nextProperty = QQmlJSMetaProperty {}; for (const QString &bit : qAsConst(aliasExprBits)) { nextProperty = resultOwner->property(bit); diff --git a/tests/auto/qml/qmltc/QmltcTests/CMakeLists.txt b/tests/auto/qml/qmltc/QmltcTests/CMakeLists.txt index fbb5de6815..cf3cd41fa4 100644 --- a/tests/auto/qml/qmltc/QmltcTests/CMakeLists.txt +++ b/tests/auto/qml/qmltc/QmltcTests/CMakeLists.txt @@ -91,6 +91,7 @@ set(qml_sources deferredProperties_attached.qml deferredProperties_complex.qml repeaterCrash.qml + aliases.qml # support types: DefaultPropertySingleChild.qml @@ -99,6 +100,9 @@ set(qml_sources LocalWithOnCompleted.qml LocallyImported_context.qml # SingletonThing.qml + ComponentWithAlias1.qml + ComponentWithAlias2.qml + ComponentWithAlias3.qml badFile.qml ) diff --git a/tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias1.qml b/tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias1.qml new file mode 100644 index 0000000000..210cf1e159 --- /dev/null +++ b/tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias1.qml @@ -0,0 +1,8 @@ +import QtQuick + +Item { + property alias setMe: firstComponent.setMe + ComponentWithAlias2 { + id: firstComponent + } +} diff --git a/tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias2.qml b/tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias2.qml new file mode 100644 index 0000000000..818f3a464e --- /dev/null +++ b/tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias2.qml @@ -0,0 +1,8 @@ +import QtQuick + +Item { + property alias setMe: firstComponent.setMe + ComponentWithAlias3 { + id: firstComponent + } +} diff --git a/tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias3.qml b/tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias3.qml new file mode 100644 index 0000000000..87b917ad19 --- /dev/null +++ b/tests/auto/qml/qmltc/QmltcTests/ComponentWithAlias3.qml @@ -0,0 +1,5 @@ +import QtQuick + +Item { + property string setMe: "Set me!" +} diff --git a/tests/auto/qml/qmltc/QmltcTests/aliases.qml b/tests/auto/qml/qmltc/QmltcTests/aliases.qml new file mode 100644 index 0000000000..9f13f7b17a --- /dev/null +++ b/tests/auto/qml/qmltc/QmltcTests/aliases.qml @@ -0,0 +1,29 @@ +// Copyright (C) 2022 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +import QtQuick + +Item { + property alias aliasToAlias: subItem.aliasToAlias + Item { + id: subItem + property alias aliasToAlias: subsubItem.aliasToAlias + + Item { + id: subsubItem + property alias aliasToAlias: subsubsubItem.value + + Item { + id: subsubsubItem + property string value: "Hello World!" + } + } + } + + property alias aliasToOtherFile: inOtherFile.setMe + + ComponentWithAlias1 { + id: inOtherFile + } +} + diff --git a/tests/auto/qml/qmltc/tst_qmltc.cpp b/tests/auto/qml/qmltc/tst_qmltc.cpp index e085ce9f24..97486bc418 100644 --- a/tests/auto/qml/qmltc/tst_qmltc.cpp +++ b/tests/auto/qml/qmltc/tst_qmltc.cpp @@ -70,7 +70,7 @@ #include "calqlatrbits.h" #include "propertychangeandsignalhandlers.h" #include "repeatercrash.h" - +#include "aliases.h" #include "testprivateproperty.h" // Qt: @@ -2194,4 +2194,34 @@ void tst_qmltc::repeaterCrash() } } +void tst_qmltc::aliases() +{ + QQmlEngine e; + PREPEND_NAMESPACE(aliases) fromQmltc(&e); + + QQmlComponent component(&e); + component.loadUrl(QUrl("qrc:/QmltcTests/aliases.qml")); + QVERIFY2(!component.isError(), qPrintable(component.errorString())); + QScopedPointer fromComponent(component.create()); + const QString testString = u"myTestString"_s; + + QCOMPARE(fromQmltc.aliasToAlias(), u"Hello World!"_s); + QCOMPARE(fromComponent->property("aliasToAlias"), u"Hello World!"_s); + + fromQmltc.setAliasToAlias(testString); + QVERIFY(fromComponent->setProperty("aliasToAlias", testString)); + + QCOMPARE(fromQmltc.aliasToAlias(), testString); + QCOMPARE(fromComponent->property("aliasToAlias"), testString); + + QCOMPARE(fromQmltc.aliasToOtherFile(), u"Set me!"_s); + QCOMPARE(fromComponent->property("aliasToOtherFile"), u"Set me!"_s); + + fromQmltc.setAliasToOtherFile(testString); + QVERIFY(fromComponent->setProperty("aliasToOtherFile", testString)); + + QCOMPARE(fromQmltc.aliasToOtherFile(), testString); + QCOMPARE(fromComponent->property("aliasToOtherFile"), testString); +} + QTEST_MAIN(tst_qmltc) diff --git a/tests/auto/qml/qmltc/tst_qmltc.h b/tests/auto/qml/qmltc/tst_qmltc.h index d0ead7ec03..bbc3be73a7 100644 --- a/tests/auto/qml/qmltc/tst_qmltc.h +++ b/tests/auto/qml/qmltc/tst_qmltc.h @@ -83,4 +83,5 @@ private slots: void calqlatrBits(); // corner cases from calqlatr demo void trickyPropertyChangeAndSignalHandlers(); void repeaterCrash(); + void aliases(); }; -- cgit v1.2.3