QtQml: Make QQmlNullableValue somewhat safer

We need it movable, and you shouldn't be able to poke its internals.

Change-Id: Ia9fbde26ef525837402d9214aa534eeec0221e2f
Reviewed-by: Sami Shalayel <sami.shalayel@qt.io>
Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io>

8 files changed, 117 insertions, 60 deletions

diff --git a/src/qml/jsruntime/qv4executablecompilationunit_p.h b/src/qml/jsruntime/qv4executablecompilationunit_p.h
index 747f776f4a..4cd1ce204e 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit_p.h
+++ b/src/qml/jsruntime/qv4executablecompilationunit_p.h
@@ -99,10 +99,16 @@ public:
     // finalUrl() and finalUrlString() shall be used to resolve further URLs referred to in the code
     // They are _not_ intercepted and thus represent the "logical" name for the code.
 
-    QUrl url() const { if (m_url.isNull) m_url = QUrl(fileName()); return m_url; }
+    QUrl url() const
+    {
+        if (!m_url.isValid())
+            m_url = QUrl(fileName());
+        return m_url;
+    }
+
     QUrl finalUrl() const
     {
-        if (m_finalUrl.isNull)
+        if (!m_finalUrl.isValid())
             m_finalUrl = QUrl(finalUrlString());
         return m_finalUrl;
     }
diff --git a/src/qml/qml/ftw/qqmlnullablevalue_p.h b/src/qml/qml/ftw/qqmlnullablevalue_p.h
index 2c1b94b556..4c80b864c0 100644
--- a/src/qml/qml/ftw/qqmlnullablevalue_p.h
+++ b/src/qml/qml/ftw/qqmlnullablevalue_p.h
@@ -22,22 +22,69 @@ QT_BEGIN_NAMESPACE
 template<typename T>
 struct QQmlNullableValue
 {
-    QQmlNullableValue()
-    : value(T()) {}
+    QQmlNullableValue() = default;
+
     QQmlNullableValue(const QQmlNullableValue<T> &o)
-    : isNull(o.isNull), value(o.value) {}
+        : m_value(o.m_value)
+        , m_isNull(o.m_isNull)
+    {}
+
+    QQmlNullableValue(QQmlNullableValue<T> &&o)
+        : m_value(std::move(o.value))
+        , m_isNull(std::exchange(o.m_isNull, true))
+    {}
+
     QQmlNullableValue(const T &t)
-    : isNull(false), value(t) {}
-    QQmlNullableValue<T> &operator=(const T &t)
-    { isNull = false; value = t; return *this; }
+        : m_value(t)
+        , m_isNull(false)
+    {}
+
+    QQmlNullableValue(T &&t)
+        : m_value(std::move(t))
+        , m_isNull(false)
+    {}
+
     QQmlNullableValue<T> &operator=(const QQmlNullableValue<T> &o)
-    { isNull = o.isNull; value = o.value; return *this; }
-    operator T() const { return value; }
+    {
+        if (&o != this) {
+            m_value = o.m_value;
+            m_isNull = o.m_isNull;
+        }
+        return *this;
+    }
+
+    QQmlNullableValue<T> &operator=(QQmlNullableValue<T> &&o)
+    {
+        if (&o != this) {
+            m_value = std::move(o.m_value);
+            m_isNull = std::exchange(o.m_isNull, true);
+        }
+        return *this;
+    }
+
+    QQmlNullableValue<T> &operator=(const T &t)
+    {
+        m_value = t;
+        m_isNull = false;
+        return *this;
+    }
+
+    QQmlNullableValue<T> &operator=(T &&t)
+    {
+        m_value = std::move(t);
+        m_isNull = false;
+        return *this;
+    }
+
+    const T &value() const { return m_value; }
+    operator T() const { return m_value; }
+
+    void invalidate() { m_isNull = true; }
+    bool isValid() const { return !m_isNull; }
 
-    void invalidate() { isNull = true; }
-    bool isValid() const { return !isNull; }
-    bool isNull = true;
-    T value;
+private:
+    T m_value = T();
+    bool m_isNull = true;
 };
 
 QT_END_NAMESPACE
diff --git a/src/quick/items/qquickitemanimation.cpp b/src/quick/items/qquickitemanimation.cpp
index 51fd2a3588..8bccc7f4cb 100644
--- a/src/quick/items/qquickitemanimation.cpp
+++ b/src/quick/items/qquickitemanimation.cpp
@@ -770,13 +770,13 @@ void QQuickPathAnimation::setOrientationExitDuration(int duration)
 qreal QQuickPathAnimation::endRotation() const
 {
     Q_D(const QQuickPathAnimation);
-    return d->endRotation.isNull ? qreal(0) : d->endRotation.value;
+    return d->endRotation.isValid() ? d->endRotation.value() : qreal(0);
 }
 
 void QQuickPathAnimation::setEndRotation(qreal rotation)
 {
     Q_D(QQuickPathAnimation);
-    if (!d->endRotation.isNull && d->endRotation == rotation)
+    if (d->endRotation.isValid() && d->endRotation == rotation)
         return;
 
     d->endRotation = rotation;
@@ -946,11 +946,11 @@ void QQuickPathAnimationUpdater::setValue(qreal v)
             //shortest distance to correct orientation
             qreal diff = angle - startRotation;
             while (diff > 180.0) {
-                startRotation.value += 360.0;
+                startRotation = startRotation.value() + 360.0;
                 diff -= 360.0;
             }
             while (diff < -180.0) {
-                startRotation.value -= 360.0;
+                startRotation = startRotation.value() - 360.0;
                 diff += 360.0;
             }
         }
diff --git a/src/quick/items/qquickmousearea.cpp b/src/quick/items/qquickmousearea.cpp
index 6fc4b6eb3f..9c83183eb7 100644
--- a/src/quick/items/qquickmousearea.cpp
+++ b/src/quick/items/qquickmousearea.cpp
@@ -1039,7 +1039,7 @@ void QQuickMouseArea::geometryChange(const QRectF &newGeometry, const QRectF &ol
     Q_D(QQuickMouseArea);
     QQuickItem::geometryChange(newGeometry, oldGeometry);
 
-    if (d->lastScenePos.isNull)
+    if (!d->lastScenePos.isValid())
         d->lastScenePos = mapToScene(d->lastPos);
     else if (newGeometry.x() != oldGeometry.x() || newGeometry.y() != oldGeometry.y())
         d->lastPos = mapFromScene(d->lastScenePos);
diff --git a/src/quick/items/qquickstateoperations.cpp b/src/quick/items/qquickstateoperations.cpp
index 1431b5ee5e..35b29b9134 100644
--- a/src/quick/items/qquickstateoperations.cpp
+++ b/src/quick/items/qquickstateoperations.cpp
@@ -159,7 +159,7 @@ QQuickParentChange::QQuickParentChange(QObject *parent)
 QQmlScriptString QQuickParentChange::x() const
 {
     Q_D(const QQuickParentChange);
-    return d->xString.value;
+    return d->xString.value();
 }
 
 void QQuickParentChange::setX(const QQmlScriptString &x)
@@ -177,7 +177,7 @@ bool QQuickParentChange::xIsSet() const
 QQmlScriptString QQuickParentChange::y() const
 {
     Q_D(const QQuickParentChange);
-    return d->yString.value;
+    return d->yString.value();
 }
 
 void QQuickParentChange::setY(const QQmlScriptString &y)
@@ -195,7 +195,7 @@ bool QQuickParentChange::yIsSet() const
 QQmlScriptString QQuickParentChange::width() const
 {
     Q_D(const QQuickParentChange);
-    return d->widthString.value;
+    return d->widthString.value();
 }
 
 void QQuickParentChange::setWidth(const QQmlScriptString &width)
@@ -213,7 +213,7 @@ bool QQuickParentChange::widthIsSet() const
 QQmlScriptString QQuickParentChange::height() const
 {
     Q_D(const QQuickParentChange);
-    return d->heightString.value;
+    return d->heightString.value();
 }
 
 void QQuickParentChange::setHeight(const QQmlScriptString &height)
@@ -231,7 +231,7 @@ bool QQuickParentChange::heightIsSet() const
 QQmlScriptString QQuickParentChange::scale() const
 {
     Q_D(const QQuickParentChange);
-    return d->scaleString.value;
+    return d->scaleString.value();
 }
 
 void QQuickParentChange::setScale(const QQmlScriptString &scale)
@@ -249,7 +249,7 @@ bool QQuickParentChange::scaleIsSet() const
 QQmlScriptString QQuickParentChange::rotation() const
 {
     Q_D(const QQuickParentChange);
-    return d->rotationString.value;
+    return d->rotationString.value();
 }
 
 void QQuickParentChange::setRotation(const QQmlScriptString &rotation)
@@ -316,13 +316,14 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
 
     if (d->xString.isValid()) {
         bool ok = false;
-        qreal x = d->xString.value.numberLiteral(&ok);
+        qreal x = d->xString.value().numberLiteral(&ok);
         if (ok) {
             QQuickStateAction xa(d->target, QLatin1String("x"), x);
             actions << xa;
         } else {
             QQmlProperty property(d->target, QLatin1String("x"));
-            auto newBinding = QQmlAnyBinding::createFromScriptString(property, d->xString.value, d->target, qmlContext(this));
+            auto newBinding = QQmlAnyBinding::createFromScriptString(
+                    property, d->xString.value(), d->target, qmlContext(this));
             QQuickStateAction xa;
             xa.property = property;
             xa.toBinding = newBinding;
@@ -334,13 +335,14 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
 
     if (d->yString.isValid()) {
         bool ok = false;
-        qreal y = d->yString.value.numberLiteral(&ok);
+        qreal y = d->yString.value().numberLiteral(&ok);
         if (ok) {
             QQuickStateAction ya(d->target, QLatin1String("y"), y);
             actions << ya;
         } else {
             QQmlProperty property(d->target, QLatin1String("y"));
-            auto newBinding = QQmlAnyBinding::createFromScriptString(property, d->yString.value, d->target, qmlContext(this));
+            auto newBinding = QQmlAnyBinding::createFromScriptString(
+                    property, d->yString.value(), d->target, qmlContext(this));
             QQuickStateAction ya;
             ya.property = property;
             ya.toBinding = newBinding;
@@ -352,13 +354,14 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
 
     if (d->scaleString.isValid()) {
         bool ok = false;
-        qreal scale = d->scaleString.value.numberLiteral(&ok);
+        qreal scale = d->scaleString.value().numberLiteral(&ok);
         if (ok) {
             QQuickStateAction sa(d->target, QLatin1String("scale"), scale);
             actions << sa;
         } else {
             QQmlProperty property(d->target, QLatin1String("scale"));
-            auto newBinding = QQmlAnyBinding::createFromScriptString(property, d->scaleString.value, d->target, qmlContext(this));
+            auto newBinding = QQmlAnyBinding::createFromScriptString(
+                    property, d->scaleString.value(), d->target, qmlContext(this));
             QQuickStateAction sa;
             sa.property = property;
             sa.toBinding = newBinding;
@@ -370,13 +373,14 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
 
     if (d->rotationString.isValid()) {
         bool ok = false;
-        qreal rotation = d->rotationString.value.numberLiteral(&ok);
+        qreal rotation = d->rotationString.value().numberLiteral(&ok);
         if (ok) {
             QQuickStateAction ra(d->target, QLatin1String("rotation"), rotation);
             actions << ra;
         } else {
             QQmlProperty property(d->target, QLatin1String("rotation"));
-            auto newBinding = QQmlAnyBinding::createFromScriptString(property, d->rotationString.value, d->target, qmlContext(this));
+            auto newBinding = QQmlAnyBinding::createFromScriptString(
+                    property, d->rotationString.value(), d->target, qmlContext(this));
             QQuickStateAction ra;
             ra.property = property;
             ra.toBinding = newBinding;
@@ -388,7 +392,7 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
 
     if (d->widthString.isValid()) {
         bool ok = false;
-        qreal width = d->widthString.value.numberLiteral(&ok);
+        qreal width = d->widthString.value().numberLiteral(&ok);
         if (ok) {
             QQuickStateAction wa(d->target, QLatin1String("width"), width);
             actions << wa;
@@ -406,7 +410,7 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
 
     if (d->heightString.isValid()) {
         bool ok = false;
-        qreal height = d->heightString.value.numberLiteral(&ok);
+        qreal height = d->heightString.value().numberLiteral(&ok);
         if (ok) {
             QQuickStateAction ha(d->target, QLatin1String("height"), height);
             actions << ha;
diff --git a/src/quick/util/qquickanimation.cpp b/src/quick/util/qquickanimation.cpp
index 67c77ae704..12208f375d 100644
--- a/src/quick/util/qquickanimation.cpp
+++ b/src/quick/util/qquickanimation.cpp
@@ -1188,7 +1188,7 @@ QVariant QQuickPropertyAction::value() const
 void QQuickPropertyAction::setValue(const QVariant &v)
 {
     Q_D(QQuickPropertyAction);
-    if (d->value.isNull || d->value != v) {
+    if (!d->value.isValid() || d->value != v) {
         d->value = v;
         emit valueChanged(v);
     }
diff --git a/src/quick/util/qquickpath.cpp b/src/quick/util/qquickpath.cpp
index e0bcd14c2e..29c09abee2 100644
--- a/src/quick/util/qquickpath.cpp
+++ b/src/quick/util/qquickpath.cpp
@@ -143,7 +143,7 @@ QQuickPath::~QQuickPath()
 qreal QQuickPath::startX() const
 {
     Q_D(const QQuickPath);
-    return d->startX.isNull ? 0 : d->startX.value;
+    return d->startX.isValid() ? d->startX.value() : 0;
 }
 
 void QQuickPath::setStartX(qreal x)
@@ -165,7 +165,7 @@ bool QQuickPath::hasStartX() const
 qreal QQuickPath::startY() const
 {
     Q_D(const QQuickPath);
-    return d->startY.isNull ? 0 : d->startY.value;
+    return d->startY.isValid() ? d->startY.value() : 0;
 }
 
 void QQuickPath::setStartY(qreal y)
@@ -408,8 +408,8 @@ QPainterPath QQuickPath::createPath(const QPointF &startPoint, const QPointF &en
         first.values[attributes.at(ii)] = 0;
     attributePoints << first;
 
-    qreal startX = d->startX.isValid() ? d->startX.value : startPoint.x();
-    qreal startY = d->startY.isValid() ? d->startY.value : startPoint.y();
+    qreal startX = d->startX.isValid() ? d->startX.value() : startPoint.x();
+    qreal startY = d->startY.isValid() ? d->startY.value() : startPoint.y();
     path.moveTo(startX, startY);
 
     const QString percentString = QStringLiteral("_qfx_percent");
@@ -493,8 +493,8 @@ QPainterPath QQuickPath::createShapePath(const QPointF &startPoint, const QPoint
 
     QPainterPath path;
 
-    qreal startX = d->startX.isValid() ? d->startX.value : startPoint.x();
-    qreal startY = d->startY.isValid() ? d->startY.value : startPoint.y();
+    qreal startX = d->startX.isValid() ? d->startX.value() : startPoint.x();
+    qreal startY = d->startY.isValid() ? d->startY.value() : startPoint.y();
     path.moveTo(startX, startY);
 
     int index = 0;
@@ -981,12 +981,12 @@ qreal QQuickPath::attributeAt(const QString &name, qreal percent) const
 
 qreal QQuickCurve::x() const
 {
-    return _x.isNull ? 0 : _x.value;
+    return _x.isValid() ? _x.value() : 0;
 }
 
 void QQuickCurve::setX(qreal x)
 {
-    if (_x.isNull || _x != x) {
+    if (!_x.isValid() || _x != x) {
         _x = x;
         emit xChanged();
         emit changed();
@@ -1000,12 +1000,12 @@ bool QQuickCurve::hasX()
 
 qreal QQuickCurve::y() const
 {
-    return _y.isNull ? 0 : _y.value;
+    return _y.isValid() ? _y.value() : 0;
 }
 
 void QQuickCurve::setY(qreal y)
 {
-    if (_y.isNull || _y != y) {
+    if (!_y.isValid() || _y != y) {
         _y = y;
         emit yChanged();
         emit changed();
@@ -1024,7 +1024,7 @@ qreal QQuickCurve::relativeX() const
 
 void QQuickCurve::setRelativeX(qreal x)
 {
-    if (_relativeX.isNull || _relativeX != x) {
+    if (!_relativeX.isValid() || _relativeX != x) {
         _relativeX = x;
         emit relativeXChanged();
         emit changed();
@@ -1043,7 +1043,7 @@ qreal QQuickCurve::relativeY() const
 
 void QQuickCurve::setRelativeY(qreal y)
 {
-    if (_relativeY.isNull || _relativeY != y) {
+    if (!_relativeY.isValid() || _relativeY != y) {
         _relativeY = y;
         emit relativeYChanged();
         emit changed();
@@ -1402,7 +1402,7 @@ qreal QQuickPathQuad::relativeControlX() const
 
 void QQuickPathQuad::setRelativeControlX(qreal x)
 {
-    if (_relativeControlX.isNull || _relativeControlX != x) {
+    if (!_relativeControlX.isValid() || _relativeControlX != x) {
         _relativeControlX = x;
         emit relativeControlXChanged();
         emit changed();
@@ -1421,7 +1421,7 @@ qreal QQuickPathQuad::relativeControlY() const
 
 void QQuickPathQuad::setRelativeControlY(qreal y)
 {
-    if (_relativeControlY.isNull || _relativeControlY != y) {
+    if (!_relativeControlY.isValid() || _relativeControlY != y) {
         _relativeControlY = y;
         emit relativeControlYChanged();
         emit changed();
@@ -1586,7 +1586,7 @@ qreal QQuickPathCubic::relativeControl1X() const
 
 void QQuickPathCubic::setRelativeControl1X(qreal x)
 {
-    if (_relativeControl1X.isNull || _relativeControl1X != x) {
+    if (!_relativeControl1X.isValid() || _relativeControl1X != x) {
         _relativeControl1X = x;
         emit relativeControl1XChanged();
         emit changed();
@@ -1605,7 +1605,7 @@ qreal QQuickPathCubic::relativeControl1Y() const
 
 void QQuickPathCubic::setRelativeControl1Y(qreal y)
 {
-    if (_relativeControl1Y.isNull || _relativeControl1Y != y) {
+    if (!_relativeControl1Y.isValid() || _relativeControl1Y != y) {
         _relativeControl1Y = y;
         emit relativeControl1YChanged();
         emit changed();
@@ -1624,7 +1624,7 @@ qreal QQuickPathCubic::relativeControl2X() const
 
 void QQuickPathCubic::setRelativeControl2X(qreal x)
 {
-    if (_relativeControl2X.isNull || _relativeControl2X != x) {
+    if (!_relativeControl2X.isValid() || _relativeControl2X != x) {
         _relativeControl2X = x;
         emit relativeControl2XChanged();
         emit changed();
@@ -1643,7 +1643,7 @@ qreal QQuickPathCubic::relativeControl2Y() const
 
 void QQuickPathCubic::setRelativeControl2Y(qreal y)
 {
-    if (_relativeControl2Y.isNull || _relativeControl2Y != y) {
+    if (!_relativeControl2Y.isValid() || _relativeControl2Y != y) {
         _relativeControl2Y = y;
         emit relativeControl2YChanged();
         emit changed();
diff --git a/src/quicktemplates/qquickmenu.cpp b/src/quicktemplates/qquickmenu.cpp
index e0f6337b60..1837fe0798 100644
--- a/src/quicktemplates/qquickmenu.cpp
+++ b/src/quicktemplates/qquickmenu.cpp
@@ -1307,10 +1307,10 @@ void QQuickMenu::popup(QQuickItem *menuItem)
 #endif
 
     // As a fallback, center the menu over its parent item.
-    if (pos.isNull && d->parentItem)
+    if (!pos.isValid() && d->parentItem)
         pos = QPointF((d->parentItem->width() - width()) / 2, (d->parentItem->height() - height()) / 2);
 
-    popup(pos.isNull ? QPointF() : pos.value, menuItem);
+    popup(pos.isValid() ? pos.value() : QPointF(), menuItem);
 }
 
 void QQuickMenu::popup(const QPointF &pos, QQuickItem *menuItem)
@@ -1411,7 +1411,7 @@ void QQuickMenu::popup(QQmlV4Function *args)
             pos = QPointF(xArg->asDouble(), yArg->asDouble());
     }
 
-    if (pos.isNull && (len >= 2 || (!parentItem && len >= 1))) {
+    if (!pos.isValid() && (len >= 2 || (!parentItem && len >= 1))) {
         // point pos
         QV4::ScopedValue posArg(scope, (*args)[parentItem ? 1 : 0]);
         const QVariant var = QV4::ExecutionEngine::toVariant(posArg, QMetaType {});
@@ -1422,10 +1422,10 @@ void QQuickMenu::popup(QQmlV4Function *args)
     if (parentItem)
         setParentItem(parentItem);
 
-    if (pos.isNull)
-        popup(menuItem);
-    else
+    if (pos.isValid())
         popup(pos, menuItem);
+    else
+        popup(menuItem);
 }
 
 /*!