LibJS: Use SortIndexedProperties AO in Array.prototype.sort

By aligning Array.prototype.sort with the spec, this removes the direct
invocation of CompareArrayElements from array_merge_sort. This opens the
door for TypedArray to use SortIndexedProperties as well because now
array_merge_sort does not assume what kind of array it is invoked with.
Further, this addresses a FIXME to avoid an extra JS heap allocation.

1 files changed, 7 insertions, 7 deletions

diff --git a/Userland/Libraries/LibJS/Runtime/Array.cpp b/Userland/Libraries/LibJS/Runtime/Array.cpp
index 6d88ccf103..79d983df90 100644
--- a/Userland/Libraries/LibJS/Runtime/Array.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Array.cpp
@@ -250,13 +250,13 @@ ThrowCompletionOr<MarkedVector<Value>> sort_indexed_properties(GlobalObject& glo
     }
 
     // 4. Sort items using an implementation-defined sequence of calls to SortCompare. If any such call returns an abrupt completion, stop before performing any further calls to SortCompare or steps in this algorithm and return that Completion Record.
-    // FIXME: Support AK::Function in array_merge_sort() to avoid having to create a NativeFunction.
-    auto* native_comparefn = NativeFunction::create(global_object, "", [&](auto& vm, auto&) -> ThrowCompletionOr<Value> {
-        auto x = vm.argument(0);
-        auto y = vm.argument(1);
-        return TRY(sort_compare(x, y));
-    });
-    TRY(array_merge_sort(global_object, native_comparefn, items));
+
+    // Perform sorting by merge sort. This isn't as efficient compared to quick sort, but
+    // quicksort can't be used in all cases because the spec requires Array.prototype.sort()
+    // to be stable. FIXME: when initially scanning through the array, maintain a flag
+    // for if an unstable sort would be indistinguishable from a stable sort (such as just
+    // just strings or numbers), and in that case use quick sort instead for better performance.
+    TRY(array_merge_sort(global_object, sort_compare, items));
 
     // 5. Return items.
     return items;