From 25d72fef689d2f0d18beaa6dceaef9f6b622d6b5 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Tue, 21 Apr 2026 15:17:49 +0300 Subject: [PATCH] builtin: fix sort() behaving differently on linux and windows (fixes #16512) --- vlib/builtin/array.v | 4 +++ vlib/builtin/array_test.v | 46 +++++++++++++++++++++++++ vlib/builtin/cfns_wrapper.c.v | 59 +++++++++++++++++++++++++++++++-- vlib/builtin/fixed_array_test.v | 15 +++++++++ vlib/v/gen/c/array.v | 37 +++++++++++++-------- 5 files changed, 145 insertions(+), 16 deletions(-) diff --git a/vlib/builtin/array.v b/vlib/builtin/array.v index 6acc402de..4af0540a8 100644 --- a/vlib/builtin/array.v +++ b/vlib/builtin/array.v @@ -1163,6 +1163,7 @@ pub fn (a array) map(callback fn (voidptr) voidptr) array // sort can take a boolean test expression as its single argument. // The expression uses 2 'magic' variables `a` and `b` as pointers to the two elements // being compared. +// Equal elements keep their original relative order. // // Example: mut aa := [5,2,1,10]; aa.sort(); assert aa == [1,2,5,10] // will sort the array in ascending order // Example: mut aa := [5,2,1,10]; aa.sort(b < a); assert aa == [10,5,2,1] // will sort the array in descending order @@ -1171,6 +1172,7 @@ pub fn (mut a array) sort(callback fn (voidptr, voidptr) int) // sorted returns a sorted copy of the original array. The original array is *NOT* modified. // See also .sort() . +// Equal elements keep their original relative order. // Example: assert [9,1,6,3,9].sorted() == [1,3,6,9,9] // Example: assert [9,1,6,3,9].sorted(b < a) == [9,9,6,3,1] pub fn (a &array) sorted(callback fn (voidptr, voidptr) int) array @@ -1182,6 +1184,7 @@ pub fn (a &array) sorted(callback fn (voidptr, voidptr) int) array // - `-1` when `a` should come before `b` ( `a < b` ) // - `1` when `b` should come before `a` ( `b < a` ) // - `0` when the order cannot be determined ( `a == b` ) +// Returning `0` keeps equal elements in their original relative order. // // Example: // ```v @@ -1208,6 +1211,7 @@ pub fn (mut a array) sort_with_compare(callback FnSortCB) { // sorted_with_compare sorts a clone of the array. The original array is not modified. // It uses the results of the given function to determine sort order. // See also .sort_with_compare() +// Equal elements keep their original relative order. pub fn (a &array) sorted_with_compare(callback FnSortCB) array { mut r := a.clone() unsafe { vqsort(r.data, usize(r.len), usize(r.element_size), callback) } diff --git a/vlib/builtin/array_test.v b/vlib/builtin/array_test.v index 2c8eba762..30d0eb3cc 100644 --- a/vlib/builtin/array_test.v +++ b/vlib/builtin/array_test.v @@ -989,6 +989,21 @@ fn test_sort() { assert users[2].name == 'Peter' } +fn test_sort_preserves_relative_order_for_equal_elements() { + source := [User{4, 'B'}, User{4, 'A'}, User{5, 'C'}] + + mut sorted := source.clone() + sorted.sort(a.age > b.age) + assert sorted[0].name == 'C' + assert sorted[1].name == 'B' + assert sorted[2].name == 'A' + + copy := source.sorted(a.age > b.age) + assert copy[0].name == 'C' + assert copy[1].name == 'B' + assert copy[2].name == 'A' +} + fn test_sort_with_compare() { mut a := ['hi', '1', '5', '3'] a.sort_with_compare(fn (a &string, b &string) int { @@ -1003,6 +1018,37 @@ fn test_sort_with_compare() { assert a == ['1', '3', '5', 'hi'] } +fn test_sort_with_compare_preserves_relative_order_for_equal_elements() { + source := [User{4, 'B'}, User{4, 'A'}, User{5, 'C'}] + + mut sorted := source.clone() + sorted.sort_with_compare(fn (a &User, b &User) int { + if a.age > b.age { + return -1 + } + if a.age < b.age { + return 1 + } + return 0 + }) + assert sorted[0].name == 'C' + assert sorted[1].name == 'B' + assert sorted[2].name == 'A' + + copy := source.sorted_with_compare(fn (a &User, b &User) int { + if a.age > b.age { + return -1 + } + if a.age < b.age { + return 1 + } + return 0 + }) + assert copy[0].name == 'C' + assert copy[1].name == 'B' + assert copy[2].name == 'A' +} + fn test_rune_sort() { mut bs := [`f`, `e`, `d`, `b`, `c`, `a`] bs.sort() diff --git a/vlib/builtin/cfns_wrapper.c.v b/vlib/builtin/cfns_wrapper.c.v index f7fe9718b..a5bdf72ee 100644 --- a/vlib/builtin/cfns_wrapper.c.v +++ b/vlib/builtin/cfns_wrapper.c.v @@ -103,13 +103,45 @@ pub fn vmemset(s voidptr, c int, n isize) voidptr { type FnSortCB = fn (const_a voidptr, const_b voidptr) int +@[inline; unsafe] +fn vsort_ptr_at(base voidptr, index usize, size usize) voidptr { + return unsafe { voidptr(&u8(base) + index * size) } +} + +@[unsafe] +fn vstable_sort_merge(source voidptr, dest voidptr, left usize, mid usize, right usize, size usize, sort_cb FnSortCB) { + mut left_index := left + mut right_index := mid + mut dest_index := left + for left_index < mid && right_index < right { + left_ptr := vsort_ptr_at(source, left_index, size) + right_ptr := vsort_ptr_at(source, right_index, size) + if sort_cb(left_ptr, right_ptr) <= 0 { + vmemcpy(vsort_ptr_at(dest, dest_index, size), left_ptr, isize(size)) + left_index++ + } else { + vmemcpy(vsort_ptr_at(dest, dest_index, size), right_ptr, isize(size)) + right_index++ + } + dest_index++ + } + if left_index < mid { + vmemcpy(vsort_ptr_at(dest, dest_index, size), vsort_ptr_at(source, left_index, size), + isize((mid - left_index) * size)) + } + if right_index < right { + vmemcpy(vsort_ptr_at(dest, dest_index, size), vsort_ptr_at(source, right_index, size), + isize((right - right_index) * size)) + } +} + @[inline; unsafe] fn vqsort(base voidptr, nmemb usize, size usize, sort_cb FnSortCB) { $if trace_vqsort ? { C.fprintf(C.stderr, c'vqsort base: %p, nmemb: %6uld, size: %6uld, sort_cb: %p\n', base, nmemb, size, sort_cb) } - if nmemb == 0 { + if nmemb < 2 || size == 0 { return } $if trace_vqsort_nulls ? { @@ -120,5 +152,28 @@ fn vqsort(base voidptr, nmemb usize, size usize, sort_cb FnSortCB) { print_backtrace() } } - C.qsort(base, nmemb, size, voidptr(sort_cb)) + total_size := isize(nmemb * size) + buffer := malloc(total_size) + defer { + free(buffer) + } + mut source := base + mut dest := voidptr(buffer) + mut width := usize(1) + for width < nmemb { + mut left := usize(0) + for left < nmemb { + mid := if left + width < nmemb { left + width } else { nmemb } + right := if left + width + width < nmemb { left + width + width } else { nmemb } + vstable_sort_merge(source, dest, left, mid, right, size, sort_cb) + left += width + width + } + mut tmp := source + source = dest + dest = tmp + width += width + } + if source != base { + vmemcpy(base, source, total_size) + } } diff --git a/vlib/builtin/fixed_array_test.v b/vlib/builtin/fixed_array_test.v index 56d0be277..655bc4160 100644 --- a/vlib/builtin/fixed_array_test.v +++ b/vlib/builtin/fixed_array_test.v @@ -325,6 +325,21 @@ fn test_fixed_array_sort() { assert users[2].name == 'Peter' } +fn test_fixed_array_sort_preserves_relative_order_for_equal_elements() { + mut source := [User{4, 'B'}, User{4, 'A'}, User{5, 'C'}]! + + mut sorted := source + sorted.sort(a.age > b.age) + assert sorted[0].name == 'C' + assert sorted[1].name == 'B' + assert sorted[2].name == 'A' + + copy := source.sorted(a.age > b.age) + assert copy[0].name == 'C' + assert copy[1].name == 'B' + assert copy[2].name == 'A' +} + fn test_sorted_immutable_original_should_not_change() { a := ['hi', '1', '5', '3']! b := a.sorted() diff --git a/vlib/v/gen/c/array.v b/vlib/v/gen/c/array.v index 79a17b3c5..246765c49 100644 --- a/vlib/v/gen/c/array.v +++ b/vlib/v/gen/c/array.v @@ -1213,31 +1213,40 @@ fn (mut g Gen) gen_array_sort(node ast.CallExpr) { // so they need internal linkage to avoid duplicate symbols at link time. sort_fn_visibility := if g.static_modifier != '' { g.static_modifier } else { 'VV_LOC ' } g.sort_fn_definitions.writeln('${sort_fn_visibility}int ${compare_fn}(${stype_arg}* a, ${stype_arg}* b) {') - c_condition := if comparison_type.sym.has_method('<') { + c_condition := g.array_sort_lt_condition(comparison_type, use_lambda, lambda_fn_name, + left_expr, right_expr, 'a', 'b') + reverse_condition := g.array_sort_lt_condition(comparison_type, use_lambda, lambda_fn_name, + right_expr, left_expr, 'b', 'a') + g.sort_fn_definitions.writeln('\tif (${c_condition}) return -1;') + g.sort_fn_definitions.writeln('\tif (${reverse_condition}) return 1;') + g.sort_fn_definitions.writeln('\treturn 0;') + g.sort_fn_definitions.writeln('}\n') + + // write call to the generated function + g.gen_array_sort_call(node, compare_fn, left_is_array) +} + +fn (mut g Gen) array_sort_lt_condition(comparison_type Type, use_lambda bool, lambda_fn_name string, left_expr string, right_expr string, left_arg string, right_arg string) string { + if use_lambda { + return '${lambda_fn_name}(${left_arg}, ${right_arg})' + } + if comparison_type.sym.has_method('<') { method_name := if comparison_type.sym.is_builtin() { 'builtin__${g.styp(comparison_type.typ)}__lt' } else { '${g.styp(comparison_type.typ)}__lt' } - '${method_name}(${left_expr}, ${right_expr})' - } else if comparison_type.unaliased_sym.has_method('<') { + return '${method_name}(${left_expr}, ${right_expr})' + } + if comparison_type.unaliased_sym.has_method('<') { method_name := if comparison_type.unaliased_sym.is_builtin() { 'builtin__${g.styp(comparison_type.unaliased)}__lt' } else { '${g.styp(comparison_type.unaliased)}__lt' } - '${method_name}(${left_expr}, ${right_expr})' - } else if use_lambda { - '${lambda_fn_name}(a, b)' - } else { - '${left_expr} < ${right_expr}' + return '${method_name}(${left_expr}, ${right_expr})' } - g.sort_fn_definitions.writeln('\tif (${c_condition}) return -1;') - g.sort_fn_definitions.writeln('\telse return 1;') - g.sort_fn_definitions.writeln('}\n') - - // write call to the generated function - g.gen_array_sort_call(node, compare_fn, left_is_array) + return '${left_expr} < ${right_expr}' } fn (mut g Gen) gen_array_sort_call(node ast.CallExpr, compare_fn string, is_array bool) { -- 2.39.5