Mark a possibly unusual pattern with a commentHEADdev

Generally, we should avoid having unscoped values lying around as they
could be collected by the GC.

A pattern in `qqmlglobal.cpp` makes a direct cast to `QV4::value` that
is safe due to the value being tied to a `QJSValue`.

Add a comment to the two usages of the pattern to identify that using a
naked `Value` might not always be safe.

The added comment is intended to make the general issue with `Value`s
more apparent to people with less experience working with the code-base,
make the reason the usage is safe in this case more explicit for people
who have cursory knowledge of the possible issue and, possibly, help
thinking in more correct terms when similar patterns are written in
other parts of the code-base.

Change-Id: Ie2614079aaad896eb86d9491cdf0f2d61c321e75
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

1 files changed, 8 insertions, 0 deletions

diff --git a/src/qml/qml/qqmlglobal.cpp b/src/qml/qml/qqmlglobal.cpp
index 1273067187..2e3fa5f86c 100644
--- a/src/qml/qml/qqmlglobal.cpp
+++ b/src/qml/qml/qqmlglobal.cpp
@@ -415,6 +415,10 @@ static QVariant byProperties(
 
     if (source.metaType() == QMetaType::fromType<QJSValue>()) {
         QJSValue val = source.value<QJSValue>();
+        // Generally, the GC might collect a Value at any point so that
+        // a `ScopedValue` should be used.
+        // In this case, the Value is tied to a `QJSValue` which is
+        // persistent to the GC and thus the cast is safe.
         return byProperties(
             targetMetaObject, targetMetaType, QV4::Value(QJSValuePrivate::asReturnedValue(&val)));
     }
@@ -560,6 +564,10 @@ bool QQmlValueTypeProvider::populateValueType(
 {
     if (sourceMetaType == QMetaType::fromType<QJSValue>()) {
         const QJSValue *val = static_cast<const QJSValue *>(source);
+        // Generally, the GC might collect a Value at any point so that
+        // a `ScopedValue` should be used.
+        // In this case, the Value is tied to a `QJSValue` which is
+        // persistent to the GC and thus the cast is safe.
         return populateValueType(
             targetMetaType, target, QV4::Value(QJSValuePrivate::asReturnedValue(val)));
     }