qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Set bindings on QQmlJSScope after AST traversal 

We can get into situations when binding creation is problematic
due to the relevant scopes being yet unresolved. In particular,
this happens when processing attached/group properties script
bindings

Avoid having this situation by postponing the actual binding
setting until after the relavant scopes are resolved (mainly,
the binding owner). However, do relevant AST order dependent
operations beforehand to avoid accidental errors

This commit amends 25098b7a4f

Fixes: QTBUG-103897
Change-Id: I671955dbe321d03e5f1ab9891cc79dc0a936deda
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Andrei Golubev 2022-05-25 15:18:56 +02:00
parent 87053c5422
commit e5251efe97
4 changed files with 153 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

130
src/qmlcompiler/qqmljsimportvisitor.cpp
View File

@ -426,6 +426,7 @@ void QQmlJSImportVisitor::endVisit(UiProgram *)
for (const auto &scope : m_objectDefinitionScopes)
checkGroupedAndAttachedScopes(scope);
setAllBindings();
processDefaultProperties();
processPropertyTypes();
processPropertyBindings();
@ -505,6 +506,25 @@ QVector<QQmlJSAnnotation> QQmlJSImportVisitor::parseAnnotations(QQmlJS::AST::UiA
return annotationList;
}
void QQmlJSImportVisitor::setAllBindings()
{
for (auto it = m_bindings.cbegin(); it != m_bindings.cend(); ++it) {
// ensure the scope is resolved, if not - it is an error
auto type = it->owner;
if (!type->isFullyResolved()) {
if (!type->isInCustomParserParent()) { // special otherwise
m_logger->log(QStringLiteral("'%1' is used but it is not resolved")
.arg(getScopeName(type, type->scopeType())),
Log_Type, type->sourceLocation());
}
continue;
}
auto binding = it->create();
if (binding.isValid())
type->addOwnPropertyBinding(binding, it->specifier);
}
}
void QQmlJSImportVisitor::processDefaultProperties()
{
for (auto it = m_pendingDefaultProperties.constBegin();
@ -1052,8 +1072,8 @@ void QQmlJSImportVisitor::addDefaultProperties()
QQmlJSMetaPropertyBinding binding(m_currentScope->sourceLocation(), defaultPropertyName);
binding.setObject(getScopeName(m_currentScope, QQmlJSScope::QMLScope),
QQmlJSScope::ConstPtr(m_currentScope));
m_currentScope->parentScope()->addOwnPropertyBinding(binding,
QQmlJSScope::UnnamedPropertyTarget);
m_bindings.append(UnfinishedBinding { m_currentScope->parentScope(), [=]() { return binding; },
QQmlJSScope::UnnamedPropertyTarget });
}
void QQmlJSImportVisitor::breakInheritanceCycles(const QQmlJSScope::Ptr &originalScope)
@ -1344,8 +1364,9 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::StringLiteral *sl)
return true;
}
inline void createNonUniqueScopeBinding(QQmlJSScope::Ptr &scope, const QString &name,
const QQmlJS::SourceLocation &srcLocation);
inline QQmlJSImportVisitor::UnfinishedBinding
createNonUniqueScopeBinding(QQmlJSScope::Ptr &scope, const QString &name,
const QQmlJS::SourceLocation &srcLocation);
bool QQmlJSImportVisitor::visit(UiObjectDefinition *definition)
{
@ -1378,7 +1399,8 @@ bool QQmlJSImportVisitor::visit(UiObjectDefinition *definition)
enterEnvironmentNonUnique(QQmlJSScope::GroupedPropertyScope, superType,
definition->firstSourceLocation());
Q_ASSERT(rootScopeIsValid());
createNonUniqueScopeBinding(m_currentScope, superType, definition->firstSourceLocation());
m_bindings.append(createNonUniqueScopeBinding(m_currentScope, superType,
definition->firstSourceLocation()));
QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports, &m_usedTypes);
}
@ -1694,7 +1716,7 @@ QQmlJSImportVisitor::parseLiteralOrScriptBinding(const QString name,
QQmlJSMetaPropertyBinding binding(location, name);
binding.setScriptBinding(addFunctionOrExpression(m_currentScope, name),
QQmlJSMetaPropertyBinding::Script_PropertyBinding);
m_currentScope->addOwnPropertyBinding(binding);
m_bindings.append(UnfinishedBinding { m_currentScope, [=]() { return binding; } });
return LiteralOrScriptParseResult::Script;
}
@ -1758,7 +1780,7 @@ QQmlJSImportVisitor::parseLiteralOrScriptBinding(const QString name,
? QQmlJSMetaPropertyBinding::ScriptValue_Undefined
: QQmlJSMetaPropertyBinding::ScriptValue_Unknown);
}
m_currentScope->addOwnPropertyBinding(binding); // always add the binding to the scope
m_bindings.append(UnfinishedBinding { m_currentScope, [=]() { return binding; } });
if (!QQmlJSMetaPropertyBinding::isLiteralBinding(binding.bindingType()))
return LiteralOrScriptParseResult::Script;
@ -1807,32 +1829,35 @@ void QQmlJSImportVisitor::handleIdDeclaration(QQmlJS::AST::UiScriptBinding *scri
The binding is added to the parentScope() of \a scope, under property name
\a name and location \a srcLocation.
*/
inline void createNonUniqueScopeBinding(QQmlJSScope::Ptr &scope, const QString &name,
const QQmlJS::SourceLocation &srcLocation)
inline QQmlJSImportVisitor::UnfinishedBinding
createNonUniqueScopeBinding(QQmlJSScope::Ptr &scope, const QString &name,
const QQmlJS::SourceLocation &srcLocation)
{
const QQmlJSScope::ScopeType type = scope->scopeType();
Q_ASSERT(type == QQmlJSScope::GroupedPropertyScope
|| type == QQmlJSScope::AttachedPropertyScope);
const QQmlJSMetaPropertyBinding::BindingType bindingType =
(type == QQmlJSScope::GroupedPropertyScope)
? QQmlJSMetaPropertyBinding::GroupProperty
: QQmlJSMetaPropertyBinding::AttachedProperty;
const auto createBinding = [=]() {
const QQmlJSScope::ScopeType type = scope->scopeType();
Q_ASSERT(type == QQmlJSScope::GroupedPropertyScope
|| type == QQmlJSScope::AttachedPropertyScope);
const QQmlJSMetaPropertyBinding::BindingType bindingType =
(type == QQmlJSScope::GroupedPropertyScope)
? QQmlJSMetaPropertyBinding::GroupProperty
: QQmlJSMetaPropertyBinding::AttachedProperty;
auto parentScope = scope->parentScope();
const auto propertyBindings = parentScope->ownPropertyBindings(name);
const bool alreadyHasBinding = std::any_of(propertyBindings.first, propertyBindings.second,
[&](const QQmlJSMetaPropertyBinding &binding) {
return binding.bindingType() == bindingType;
});
if (alreadyHasBinding)
return;
const auto propertyBindings = scope->parentScope()->ownPropertyBindings(name);
const bool alreadyHasBinding = std::any_of(propertyBindings.first, propertyBindings.second,
[&](const QQmlJSMetaPropertyBinding &binding) {
return binding.bindingType() == bindingType;
});
if (alreadyHasBinding) // no need to create any more
return QQmlJSMetaPropertyBinding(QQmlJS::SourceLocation {});
QQmlJSMetaPropertyBinding binding(srcLocation, name);
if (type == QQmlJSScope::GroupedPropertyScope)
binding.setGroupBinding(static_cast<QSharedPointer<QQmlJSScope>>(scope));
else
binding.setAttachedBinding(static_cast<QSharedPointer<QQmlJSScope>>(scope));
parentScope->addOwnPropertyBinding(std::move(binding));
QQmlJSMetaPropertyBinding binding(srcLocation, name);
if (type == QQmlJSScope::GroupedPropertyScope)
binding.setGroupBinding(static_cast<QSharedPointer<QQmlJSScope>>(scope));
else
binding.setAttachedBinding(static_cast<QSharedPointer<QQmlJSScope>>(scope));
return binding;
};
return { scope->parentScope(), createBinding };
}
bool QQmlJSImportVisitor::visit(UiScriptBinding *scriptBinding)
@ -1861,7 +1886,8 @@ bool QQmlJSImportVisitor::visit(UiScriptBinding *scriptBinding)
enterEnvironmentNonUnique(QQmlJSScope::GroupedPropertyScope, name,
group->firstSourceLocation());
}
createNonUniqueScopeBinding(m_currentScope, name, group->firstSourceLocation());
m_bindings.append(
createNonUniqueScopeBinding(m_currentScope, name, group->firstSourceLocation()));
}
auto name = group->name;
@ -1895,7 +1921,7 @@ bool QQmlJSImportVisitor::visit(UiScriptBinding *scriptBinding)
qMakePair(name.toString(), signalParameters) });
QQmlJSMetaMethod scopeSignal;
const auto methods = m_savedBindingOuterScope->methods(*signal, QQmlJSMetaMethod::Signal);
const auto methods = m_currentScope->methods(*signal, QQmlJSMetaMethod::Signal);
if (!methods.isEmpty())
scopeSignal = methods[0];
@ -1906,18 +1932,28 @@ bool QQmlJSImportVisitor::visit(UiScriptBinding *scriptBinding)
m_signalHandlers.insert(firstSourceLocation,
{ scopeSignal.parameterNames(), hasMultilineStatementBody });
// when encountering a signal handler, add it as a script binding
QQmlJSMetaPropertyBinding::ScriptBindingKind kind =
QQmlJSMetaPropertyBinding::Script_Invalid;
if (!methods.isEmpty())
kind = QQmlJSMetaPropertyBinding::Script_SignalHandler;
else if (propertyForChangeHandler(m_savedBindingOuterScope, *signal).has_value())
kind = QQmlJSMetaPropertyBinding::Script_ChangeHandler;
const QString stringName = name.toString();
// NB: calculate runtime index right away to avoid miscalculation due to
// losing real AST traversal order
const auto index = addFunctionOrExpression(m_currentScope, stringName);
const auto createBinding = [scope = m_currentScope, signalName = *signal, index, stringName,
firstSourceLocation]() {
// when encountering a signal handler, add it as a script binding
Q_ASSERT(scope->isFullyResolved());
QQmlJSMetaPropertyBinding::ScriptBindingKind kind =
QQmlJSMetaPropertyBinding::Script_Invalid;
const auto methods = scope->methods(signalName, QQmlJSMetaMethod::Signal);
if (!methods.isEmpty())
kind = QQmlJSMetaPropertyBinding::Script_SignalHandler;
else if (propertyForChangeHandler(scope, signalName).has_value())
kind = QQmlJSMetaPropertyBinding::Script_ChangeHandler;
// ### TODO: needs an error if kind is still Invalid
QString stringName = name.toString();
QQmlJSMetaPropertyBinding binding(firstSourceLocation, stringName);
binding.setScriptBinding(addFunctionOrExpression(m_currentScope, stringName), kind);
m_currentScope->addOwnPropertyBinding(binding);
QQmlJSMetaPropertyBinding binding(firstSourceLocation, stringName);
binding.setScriptBinding(index, kind);
return binding;
};
m_bindings.append(UnfinishedBinding { m_currentScope, createBinding });
}
// TODO: before leaving the scopes, we must create the binding.
@ -1978,7 +2014,8 @@ void QQmlJSImportVisitor::endVisit(UiArrayBinding *arrayBinding)
element->firstSourceLocation(), false };
QQmlJSMetaPropertyBinding binding(element->firstSourceLocation(), propertyName);
binding.setObject(getScopeName(type, QQmlJSScope::QMLScope), QQmlJSScope::ConstPtr(type));
m_currentScope->addOwnPropertyBinding(binding, QQmlJSScope::ListPropertyTarget);
m_bindings.append(UnfinishedBinding { m_currentScope, [=]() { return binding; },
QQmlJSScope::ListPropertyTarget });
}
}
@ -2265,7 +2302,8 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::UiObjectBinding *uiob)
bool exists = enterEnvironmentNonUnique(scopeKind, idName, group->firstSourceLocation());
createNonUniqueScopeBinding(m_currentScope, idName, group->firstSourceLocation());
m_bindings.append(
createNonUniqueScopeBinding(m_currentScope, idName, group->firstSourceLocation()));
++scopesEnteredCounter;
needsResolution = needsResolution || !exists;
@ -2359,7 +2397,7 @@ void QQmlJSImportVisitor::endVisit(QQmlJS::AST::UiObjectBinding *uiob)
binding.setObject(getScopeName(childScope, QQmlJSScope::QMLScope),
QQmlJSScope::ConstPtr(childScope));
}
m_currentScope->addOwnPropertyBinding(binding);
m_bindings.append(UnfinishedBinding { m_currentScope, [=]() { return binding; } });
}
for (int i = 0; i < scopesEnteredCounter; ++i)

11
src/qmlcompiler/qqmljsimportvisitor_p.h
View File

@ -54,6 +54,7 @@
#include <private/qqmljsdiagnosticmessage_p.h>
#include <private/qv4compileddata_p.h>
#include <functional>
QT_BEGIN_NAMESPACE
@ -89,6 +90,13 @@ public:
QQmlJSImporter *importer() { return m_importer; } // ### should this be restricted?
struct UnfinishedBinding
{
QQmlJSScope::Ptr owner;
std::function<QQmlJSMetaPropertyBinding()> create;
QQmlJSScope::BindingTargetSpecifier specifier = QQmlJSScope::SimplePropertyTarget;
};
protected:
// Linter warnings, we might want to move this at some point
bool visit(QQmlJS::AST::StringLiteral *) override;
@ -182,6 +190,8 @@ protected:
// A set of all types that have been used during type resolution
QSet<QString> m_usedTypes;
QList<UnfinishedBinding> m_bindings;
// stores JS functions and Script bindings per scope (only the name). mimics
// the content of QmlIR::Object::functionsAndExpressions
QHash<QQmlJSScope::ConstPtr, QList<QString>> m_functionsAndExpressions;
@ -227,6 +237,7 @@ protected:
bool isTypeResolved(const QQmlJSScope::ConstPtr &type);
QVector<QQmlJSAnnotation> parseAnnotations(QQmlJS::AST::UiAnnotationList *list);
void setAllBindings();
void addDefaultProperties();
void processDefaultProperties();
void processPropertyBindings();

15
tests/auto/qml/qqmljsscope/data/resolvedNonUniqueScope.qml Normal file
View File

@ -0,0 +1,15 @@
import QtQuick
Item {
Component.onCompleted: {} // `Component` must be resolved
Component.onDestruction: {}
property IC p
p.onXChanged: {
console.log("hooray");
}
component IC : Text {
property string x
}
}

43
tests/auto/qml/qqmljsscope/tst_qqmljsscope.cpp
View File

@ -131,6 +131,7 @@ private Q_SLOTS:
void extensions();
void emptyBlockBinding();
void qualifiedName();
void resolvedNonUniqueScopes();
public:
tst_qqmljsscope()
@ -702,5 +703,47 @@ void tst_qqmljsscope::qualifiedName()
QCOMPARE(qualifiedImportTimerInItemInComponent->baseType()->moduleName(), "QtQml");
}
void tst_qqmljsscope::resolvedNonUniqueScopes()
{
QQmlJSScope::ConstPtr root = run(u"resolvedNonUniqueScope.qml"_s);
QVERIFY(root);
const auto value = [](const QMultiHash<QString, QQmlJSMetaPropertyBinding> &bindings,
const QString &key) {
return bindings.value(key, QQmlJSMetaPropertyBinding(QQmlJS::SourceLocation {}));
};
{
auto topLevelBindings = root->propertyBindings(u"Component"_s);
QCOMPARE(topLevelBindings.size(), 1);
QCOMPARE(topLevelBindings[0].bindingType(), QQmlJSMetaPropertyBinding::AttachedProperty);
auto componentScope = topLevelBindings[0].attachingType();
auto componentBindings = componentScope->ownPropertyBindings();
QCOMPARE(componentBindings.size(), 2);
auto onCompletedBinding = value(componentBindings, u"onCompleted"_s);
QVERIFY(onCompletedBinding.isValid());
QCOMPARE(onCompletedBinding.bindingType(), QQmlJSMetaPropertyBinding::Script);
QCOMPARE(onCompletedBinding.scriptKind(), QQmlJSMetaPropertyBinding::Script_SignalHandler);
auto onDestructionBinding = value(componentBindings, u"onDestruction"_s);
QVERIFY(onDestructionBinding.isValid());
QCOMPARE(onDestructionBinding.bindingType(), QQmlJSMetaPropertyBinding::Script);
QCOMPARE(onDestructionBinding.scriptKind(),
QQmlJSMetaPropertyBinding::Script_SignalHandler);
}
{
auto topLevelBindings = root->propertyBindings(u"p"_s);
QCOMPARE(topLevelBindings.size(), 1);
QCOMPARE(topLevelBindings[0].bindingType(), QQmlJSMetaPropertyBinding::GroupProperty);
auto pScope = topLevelBindings[0].groupType();
auto pBindings = pScope->ownPropertyBindings();
QCOMPARE(pBindings.size(), 1);
auto onXChangedBinding = value(pBindings, u"onXChanged"_s);
QVERIFY(onXChangedBinding.isValid());
QCOMPARE(onXChangedBinding.bindingType(), QQmlJSMetaPropertyBinding::Script);
QCOMPARE(onXChangedBinding.scriptKind(), QQmlJSMetaPropertyBinding::Script_SignalHandler);
}
}
QTEST_MAIN(tst_qqmljsscope)
#include "tst_qqmljsscope.moc"