diff options
9 files changed, 195 insertions, 7 deletions
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp index 0a50234136..47d1b5e785 100644 --- a/src/qml/qml/qqmlobjectcreator.cpp +++ b/src/qml/qml/qqmlobjectcreator.cpp @@ -922,7 +922,8 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper qmlBinding = QQmlTranslationPropertyBinding::create(bindingProperty, compilationUnit, binding); } else { QV4::Function *runtimeFunction = compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]; - qmlBinding = QQmlPropertyBinding::create(bindingProperty, runtimeFunction, _scopeObject, context, currentQmlContext()); + QQmlPropertyIndex index(bindingProperty->coreIndex(), -1); + qmlBinding = QQmlPropertyBinding::create(bindingProperty, runtimeFunction, _scopeObject, context, currentQmlContext(), _bindingTarget, index); } sharedState.data()->allQPropertyBindings.emplaceBack(_bindingTarget, bindingProperty->coreIndex(), qmlBinding); } else { diff --git a/src/qml/qml/qqmlpropertybinding.cpp b/src/qml/qml/qqmlpropertybinding.cpp index eaf3524487..313cd7f395 100644 --- a/src/qml/qml/qqmlpropertybinding.cpp +++ b/src/qml/qml/qqmlpropertybinding.cpp @@ -44,7 +44,7 @@ QT_BEGIN_NAMESPACE QUntypedPropertyBinding QQmlPropertyBinding::create(const QQmlPropertyData *pd, QV4::Function *function, QObject *obj, const QQmlRefPointer<QQmlContextData> &ctxt, - QV4::ExecutionContext *scope) + QV4::ExecutionContext *scope, QObject *target, QQmlPropertyIndex targetIndex) { if (auto aotFunction = function->aotFunction; aotFunction && aotFunction->returnType == pd->propType()) { return QUntypedPropertyBinding(aotFunction->returnType, @@ -69,7 +69,7 @@ QUntypedPropertyBinding QQmlPropertyBinding::create(const QQmlPropertyData *pd, } auto buffer = new std::byte[sizeof(QQmlPropertyBinding)]; // QQmlPropertyBinding uses delete[] - auto binding = new (buffer) QQmlPropertyBinding(QMetaType(pd->propType())); + auto binding = new(buffer) QQmlPropertyBinding(QMetaType(pd->propType()), target, targetIndex); binding->setNotifyOnValueChanged(true); binding->setContext(ctxt); binding->setScopeObject(obj); @@ -87,6 +87,7 @@ void QQmlPropertyBinding::expressionChanged() err.setLine(location.line); err.setColumn(location.column); err.setDescription(QString::fromLatin1("Binding loop detected")); + err.setObject(target()); qmlWarning(this->scopeObject(), err); return; } @@ -95,11 +96,16 @@ void QQmlPropertyBinding::expressionChanged() m_error.setTag(currentTag); } -QQmlPropertyBinding::QQmlPropertyBinding(QMetaType mt) +QQmlPropertyBinding::QQmlPropertyBinding(QMetaType mt, QObject *target, QQmlPropertyIndex targetIndex) : QPropertyBindingPrivate(mt, &QtPrivate::bindingFunctionVTable<QQmlPropertyBinding>, - QPropertyBindingSourceLocation()) + QPropertyBindingSourceLocation(), true) { + static_assert (std::is_trivially_destructible_v<TargetData>); + static_assert (sizeof(TargetData) + sizeof(DeclarativeErrorCallback) <= sizeof(QPropertyBindingSourceLocation)); + static_assert (alignof(TargetData) <= alignof(QPropertyBindingSourceLocation)); + new (&declarativeExtraData) TargetData {target, targetIndex}; + errorCallBack = bindingErrorCallback; } bool QQmlPropertyBinding::evaluate(QMetaType metaType, void *dataPtr) @@ -124,6 +130,7 @@ bool QQmlPropertyBinding::evaluate(QMetaType metaType, void *dataPtr) if (hasError()) { QPropertyBindingError error(QPropertyBindingError::UnknownError, delayedError()->error().description()); QPropertyBindingPrivate::currentlyEvaluatingBinding()->setError(std::move(error)); + bindingErrorCallback(this); return false; } @@ -194,6 +201,57 @@ bool QQmlPropertyBinding::evaluate(QMetaType metaType, void *dataPtr) return hasChanged; } +QString QQmlPropertyBinding::createBindingLoopErrorDescription(QJSEnginePrivate *ep) +{ + QQmlPropertyData *propertyData = nullptr; + QQmlPropertyData valueTypeData; + QQmlData *data = QQmlData::get(target(), false); + Q_ASSERT(data); + if (Q_UNLIKELY(!data->propertyCache)) { + data->propertyCache = ep->cache(target()->metaObject()); + data->propertyCache->addref(); + } + propertyData = data->propertyCache->property(targetIndex().coreIndex()); + Q_ASSERT(propertyData); + Q_ASSERT(!targetIndex().hasValueTypeIndex()); + QQmlProperty prop = QQmlPropertyPrivate::restore(target(), *propertyData, &valueTypeData, nullptr); + return QStringLiteral(R"(QML %1: Binding loop detected for property "%2")").arg(QQmlMetaType::prettyTypeName(target()) , prop.name()); +} + +QObject *QQmlPropertyBinding::target() +{ + return std::launder(reinterpret_cast<TargetData *>(&declarativeExtraData))->target; +} + +QQmlPropertyIndex QQmlPropertyBinding::targetIndex() +{ + return std::launder(reinterpret_cast<TargetData *>(&declarativeExtraData))->targetIndex; +} + +void QQmlPropertyBinding::bindingErrorCallback(QPropertyBindingPrivate *that) +{ + auto This = static_cast<QQmlPropertyBinding *>(that); + auto target = This->target(); + auto engine = qmlEngine(target); + if (!engine) + return; + auto ep = QQmlEnginePrivate::get(engine); + + auto error = This->bindingError(); + QQmlError qmlError; + auto location = This->QQmlJavaScriptExpression::sourceLocation(); + qmlError.setColumn(location.column); + qmlError.setLine(location.line); + qmlError.setUrl(QUrl {location.sourceFile}); + auto description = error.description(); + if (error.type() == QPropertyBindingError::BindingLoop) { + description = This->createBindingLoopErrorDescription(ep); + } + qmlError.setDescription(description); + qmlError.setObject(target); + ep->warning(qmlError); +} + QUntypedPropertyBinding QQmlTranslationPropertyBinding::create(const QQmlPropertyData *pd, const QQmlRefPointer<QV4::ExecutableCompilationUnit> &compilationUnit, const QV4::CompiledData::Binding *binding) { auto translationBinding = [compilationUnit, binding](const QMetaType &metaType, void *dataPtr) -> bool { diff --git a/src/qml/qml/qqmlpropertybinding_p.h b/src/qml/qml/qqmlpropertybinding_p.h index 1ea82b26be..4a9d1f4558 100644 --- a/src/qml/qml/qqmlpropertybinding_p.h +++ b/src/qml/qml/qqmlpropertybinding_p.h @@ -74,7 +74,8 @@ public: static QUntypedPropertyBinding create(const QQmlPropertyData *pd, QV4::Function *function, QObject *obj, const QQmlRefPointer<QQmlContextData> &ctxt, - QV4::ExecutionContext *scope); + QV4::ExecutionContext *scope, QObject *target, + QQmlPropertyIndex targetIndex); void expressionChanged() override; @@ -87,11 +88,21 @@ public: } private: - QQmlPropertyBinding(QMetaType metaType); + QQmlPropertyBinding(QMetaType metaType, QObject *target, QQmlPropertyIndex targetIndex); bool evaluate(QMetaType metaType, void *dataPtr); + QString createBindingLoopErrorDescription(QJSEnginePrivate *ep); + struct TargetData { + QObject *target; + QQmlPropertyIndex targetIndex; + }; + + QObject *target(); + QQmlPropertyIndex targetIndex(); + + static void bindingErrorCallback(QPropertyBindingPrivate *); }; template <auto I> diff --git a/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerEager.qml b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerEager.qml new file mode 100644 index 0000000000..a320ba56e6 --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerEager.qml @@ -0,0 +1,6 @@ +import test + +BindingLoop { + eager1: eager2+1 + eager2: eager1+1 +} diff --git a/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerLazy.qml b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerLazy.qml new file mode 100644 index 0000000000..aabd41e61c --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerLazy.qml @@ -0,0 +1,13 @@ +import test +import QtQml + +QtObject { + property BindingLoop a: BindingLoop { + value: b.eager1+1 + } + + property BindingLoop b: BindingLoop { + eager1: a.value+1 + } +} + diff --git a/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerOld.qml b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerOld.qml new file mode 100644 index 0000000000..88553ed4a7 --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerOld.qml @@ -0,0 +1,6 @@ +import test + +BindingLoop { + eager1: oldprop+1 + oldprop: eager1+1 +} diff --git a/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyEager.qml b/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyEager.qml new file mode 100644 index 0000000000..98fef4cbda --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyEager.qml @@ -0,0 +1,6 @@ +import test + +BindingLoop { + eager1: value+1 + value: eager1+1 +} diff --git a/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyLazy.qml b/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyLazy.qml new file mode 100644 index 0000000000..c054282f93 --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyLazy.qml @@ -0,0 +1,12 @@ +import test +import QtQml + +BindingLoop { + id: loop + value: value2 + 1 + value2: value + 1 + + Component.onCompleted: { + let x = loop.value2 // if we do not read the value, we don't detect the loop... + } +} diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp index 02d044aa63..86bd32ccbe 100644 --- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp +++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp @@ -53,6 +53,7 @@ #include <private/qv4objectiterator_p.h> #include <private/qqmlabstractbinding_p.h> #include <private/qqmlvaluetypeproxybinding_p.h> +#include <QtCore/private/qproperty_p.h> #ifdef Q_CC_MSVC #define NO_INLINE __declspec(noinline) @@ -92,6 +93,8 @@ private slots: void signalAssignment(); void signalArguments(); void bindingLoop(); + void cppPropertyBindingLoop_data(); + void cppPropertyBindingLoop(); void basicExpressions(); void basicExpressions_data(); void arrayExpressions(); @@ -883,6 +886,78 @@ void tst_qqmlecmascript::bindingLoop() delete object; } + +struct QPropertyBindingLoop : public QObject +{ + Q_OBJECT + Q_PROPERTY(float value MEMBER value BINDABLE bindableValue) + Q_PROPERTY(float value2 MEMBER value2 BINDABLE bindableValue2) + Q_PROPERTY(float eager1 READ eager1 WRITE setEager1 BINDABLE bindableEager1) + Q_PROPERTY(float eager2 READ eager2 WRITE setEager2 BINDABLE bindableEager2) + Q_PROPERTY(float oldprop READ oldprop WRITE setOldprop NOTIFY oldpropChanged) +signals: + void oldpropChanged(); +public: + + float eager1() {return eager1Data;} + void setEager1(float f) {eager1Data = f;} + float eager2() {return eager2Data;} + void setEager2(float f) {eager2Data = f;} + QProperty<float> value; + QProperty<float> value2; + QBindable<float> bindableValue() { return QBindable<float>(&value); } + QBindable<float> bindableValue2() { return QBindable<float>(&value2); } + QBindable<float> bindableEager1() {return QBindable<float>(&eager1Data);} + QBindable<float> bindableEager2() {return QBindable<float>(&eager2Data);} + float m_oldprop; + + float oldprop() const; + void setOldprop(float oldprop); + +private: + Q_OBJECT_COMPAT_PROPERTY(QPropertyBindingLoop, float, eager1Data, &QPropertyBindingLoop::setEager1); + Q_OBJECT_COMPAT_PROPERTY(QPropertyBindingLoop, float, eager2Data, &QPropertyBindingLoop::setEager2); + +}; + + +float QPropertyBindingLoop::oldprop() const +{ + return m_oldprop; +} + +void QPropertyBindingLoop::setOldprop(float oldprop) +{ + m_oldprop = oldprop; + emit oldpropChanged(); +} + +void tst_qqmlecmascript::cppPropertyBindingLoop_data() +{ + QTest::addColumn<QString>("file"); + QTest::addColumn<QString>("warningMsg"); + + QTest::newRow("eager eager") << "bindingLoopEagerEager.qml" << R"(:4:5: QML BindingLoop: Binding loop detected for property "eager1")"; + QTest::newRow("lazy lazy") << "bindingLoopLazyLazy.qml" << R"(:7:5: QML BindingLoop: Binding loop detected for property "value2")"; + QTest::newRow("lazy eager") << "bindingLoopLazyEager.qml" << R"(:4:5: QML BindingLoop: Binding loop detected for property "eager1")"; + QTest::newRow("eager lazy") << "bindingLoopEagerLazy.qml" << R"(:10:9: QML BindingLoop: Binding loop detected for property "eager1")"; + QTest::newRow("eager old") << "bindingLoopEagerOld.qml" << R"(:4:5: QML BindingLoop: Binding loop detected for property "eager1")"; + + qmlRegisterType<QPropertyBindingLoop>("test", 1, 0, "BindingLoop"); +} + +void tst_qqmlecmascript::cppPropertyBindingLoop() +{ + QFETCH(QString, file); + QFETCH(QString, warningMsg); + QQmlEngine engine; + QQmlComponent component(&engine, testFileUrl(file)); + QString warning = component.url().toString() + warningMsg; + QTest::ignoreMessage(QtWarningMsg, warning.toLatin1().constData()); + QScopedPointer<QObject> object { component.create() }; + QVERIFY2(object, qPrintable(component.errorString())); +} + void tst_qqmlecmascript::basicExpressions_data() { QTest::addColumn<QString>("expression"); |