QQmlPropertyBinding: detect binding loops

Calling markDirtyAndNotifyObservers bypasses the currently existing
mechanism in QProperty to detect binding loops. Moreover, simply
changing updating from private to protected and setting it to true does
not help us here: That would trigger a binding loop warning every time
the static callback of a QNotifiedProperty acesses its current value.
Thus, we add another bool to detect expressionChanged loops. To reduce
memory usage, we store it as a second flag in m_error (from
QQmlJavaScriptExpression).

Change-Id: If931d7ff58b5f9bb277e803826b28e133b2c15dc
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

2 files changed, 29 insertions, 9 deletions

diff --git a/src/qml/qml/qqmljavascriptexpression_p.h b/src/qml/qml/qqmljavascriptexpression_p.h
index f90b399b0a..c2df5004d1 100644
--- a/src/qml/qml/qqmljavascriptexpression_p.h
+++ b/src/qml/qml/qqmljavascriptexpression_p.h
@@ -184,23 +184,29 @@ protected:
 
     QForwardFieldList<QQmlJavaScriptExpressionGuard, &QQmlJavaScriptExpressionGuard::next, GuardTag> activeGuards;
 
-    void setTranslationsCaptured(bool captured) { if (captured) m_error.setTag(TranslationsCaptured); else m_error.setTag(NoTag); }
-    bool translationsCaptured() const { return m_error.tag() == TranslationsCaptured; }
-
-private:
-    friend class QQmlContextData;
-    friend class QQmlPropertyCapture;
-    friend void QQmlJavaScriptExpressionGuard_callback(QQmlNotifierEndpoint *, void **);
-    friend class QQmlTranslationBinding;
+    void setTranslationsCaptured(bool captured) {
+        Tag newTag = captured ? TranslationsCaptured : NoTag;
+        if (m_error.tag() & InEvaluationLoop)
+            newTag = Tag(newTag | InEvaluationLoop);
+        m_error.setTag(newTag);
+    }
+    bool translationsCaptured() const { return m_error.tag() & TranslationsCaptured; }
 
     enum Tag {
         NoTag,
-        TranslationsCaptured
+        TranslationsCaptured,
+        InEvaluationLoop
     };
 
     // m_error:flag1 translationsCapturedDuringEvaluation
     QTaggedPointer<QQmlDelayedError> m_error;
 
+private:
+    friend class QQmlContextData;
+    friend class QQmlPropertyCapture;
+    friend void QQmlJavaScriptExpressionGuard_callback(QQmlNotifierEndpoint *, void **);
+    friend class QQmlTranslationBinding;
+
     // Not refcounted as the context will clear the expressions when destructed.
     QQmlContextData *m_context;
 
diff --git a/src/qml/qml/qqmlpropertybinding.cpp b/src/qml/qml/qqmlpropertybinding.cpp
index 6b82614e6d..a528cf2754 100644
--- a/src/qml/qml/qqmlpropertybinding.cpp
+++ b/src/qml/qml/qqmlpropertybinding.cpp
@@ -38,6 +38,7 @@
 ****************************************************************************/
 
 #include "qqmlpropertybinding_p.h"
+#include <qqmlinfo.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -72,7 +73,20 @@ QUntypedPropertyBinding QQmlPropertyBinding::create(const QQmlPropertyData *pd,
 
 void QQmlPropertyBinding::expressionChanged()
 {
+    const auto currentTag = m_error.tag();
+    if (currentTag & InEvaluationLoop) {
+        QQmlError err;
+        auto location = QQmlJavaScriptExpression::sourceLocation();
+        err.setUrl(QUrl{location.sourceFile});
+        err.setLine(location.line);
+        err.setColumn(location.column);
+        err.setDescription(QString::fromLatin1("Binding loop detected"));
+        QtQml::qmlWarning(this->scopeObject(), err);
+        return;
+    }
+    m_error.setTag(currentTag | InEvaluationLoop);
     markDirtyAndNotifyObservers();
+    m_error.setTag(currentTag);
 }
 
 QQmlPropertyBinding::QQmlPropertyBinding(const QMetaType &mt)