Rework the sort implementation for Sequences

When QML reads a property with a C++ provenance it sometimes apply
certain transformations to work with the property in a JS environment.

For example, certain containers, such as `QJsonArray` or
`QVariantList`, are converted to a `Sequence`, an array-like object that
knows how to modify the container and takes care of reflecting mutations
back to the property.

`Sequence` provides a specialized implementation for the built-in sort
method.

Generally, the default sort implementation for an array in JS converts
the elements to a string and compares the stringified representation.

In the case of `Sequence`, the sort implementation will treats the
elements as `QVariant`s and use `QVariant::toString` to perform this
part of the sorting algorithm.

Due to the way `QVariant::toString` works, this can fail for certain
elements.
For example, `QVariant::toString` is unaware of how to produce a string
from a `QJsonValue`, the type of the elements that compose a
`QJsonArray`, thus failing to correctly sort a container with such
elements.

Other than the `Sequence` implementation, the JS runtime provides, as
per specification, a sort method for the Array prototype.

Contrary to other methods that are implemented for the prototype, the
`sort` method is implemented so that it can only work on values that
have a populated `ArrayData`, an optimized storage for certain array and
array-like objects.

As `Sequences` do not use an `ArrayData` storage for their elements, the
method is unable to work on a `Sequence`.

To broaden the ability of the sort method implementation for `Sequence`
to work more generically, the default sort implementation for the Array
prototype sort method was modified to work more generically on objects
that do not present an `ArrayData` storage, with an implementation based
on the latest draft of the JS specification.

The specialized `Sequence` implementation was removed, in favor of
`Sequence` delegating to the Array prototype implementation which would
now support working with `Sequence`s.

While this should be generally slower than the specialized
implementation, foregoing some performance, it should allow a more
generic foundation for the sort method for `Sequences` or other elements
that act like an array but do not use the specialized `ArrayData`
representation.
Some specialization could later be reapplied to `Sequence` to improve
the performances of the implementation.

Previously, the Array prototype implementation would directly delegate
to `ArrayData::sort`, the sort implementation for the specialized
`ArrayData` storage.
This was modified to dispatch to an implementation based on generic
methods when no `ArrayData` is populated on the object of the sort.

The code related to the specialized `Sequence` implementation for sort
was removed and the sequence prototype was modified to not present a
specialized `sort` property, so as to fallback on the Array prototype
one.

The `ArrayData::sort` implementation was slightly modified.
`ArrayData::sort` would perform a check on the presence of a defined
callback for the sorting and throw a type error if the non-undefined
element is not callable, as per specification.

This check was moved to the Array prototype implementation, to be shared
between the specialized `ArrayData::sort` implementation and the
generic implementation.

As per the spec, the check should be performed as soon as the method is
entered and before the receiver of the method is converted to an object.

With the check moved to the start of the Array prototype sort
implementation this ordering of operations is now fulfilled.
The compliance test that checks for this behavior,
`comparefn-nonfunction-call-throws`, that was previously failing, will
now pass and was thus removed from the list of expected failures for the
`ecmascript` tests.

A `QEXPECT_FAIL` related to testing the default sort of a `QJsonArray`
property was removed from `tst_qqmllanguage`, as the sort is now
expected to work correctly.

Fixes: QTBUG-125400
Change-Id: I158a9a160b8bdde2b8a06bb349a76469fc25c5a1
Reviewed-by: Ulf Hermann <[email protected]>

1 files changed, 0 insertions, 5 deletions

diff --git a/src/qml/jsruntime/qv4arraydata.cpp b/src/qml/jsruntime/qv4arraydata.cpp
index e1da807c21..724f6fbfa3 100644
--- a/src/qml/jsruntime/qv4arraydata.cpp
+++ b/src/qml/jsruntime/qv4arraydata.cpp
@@ -630,11 +630,6 @@ void ArrayData::sort(ExecutionEngine *engine, Object *thisObject, const Value &c
     if (!arrayData || !arrayData->length())
         return;
 
-    if (!comparefn.isUndefined() && !comparefn.isFunctionObject()) {
-        engine->throwTypeError();
-        return;
-    }
-
     // The spec says the sorting goes through a series of get,put and delete operations.
     // this implies that the attributes don't get sorted around.