From fe70f10bcced86e3cc1f1b19a4fee2e625e19cbe Mon Sep 17 00:00:00 2001 From: Christoffer Lerno Date: Tue, 7 Oct 2025 22:43:40 +0200 Subject: [PATCH] Sorting functions correctly took slices by value, but also other types by value. Now, only slices are accepted by value, other containers are always by ref. --- benchmarks/stdlib/crypto/aes_bench.c3 | 4 +- lib/std/sort/binarysearch.c3 | 111 ++++++++++++++++-------- lib/std/sort/countingsort.c3 | 56 +++++++++---- lib/std/sort/insertionsort.c3 | 39 +++++---- lib/std/sort/quicksort.c3 | 77 +++++++++-------- lib/std/sort/sort.c3 | 112 +++++++++++++++++++++---- lib/std/sort/sorted.c3 | 59 ++++++++++++- releasenotes.md | 1 + src/compiler/sema_expr.c | 2 +- test/unit/stdlib/collections/map.c3 | 2 +- test/unit/stdlib/sort/countingsort.c3 | 4 +- test/unit/stdlib/sort/insertionsort.c3 | 2 +- test/unit/stdlib/sort/quicksort.c3 | 2 +- 13 files changed, 346 insertions(+), 125 deletions(-) diff --git a/benchmarks/stdlib/crypto/aes_bench.c3 b/benchmarks/stdlib/crypto/aes_bench.c3 index 8e148e997..129f9df05 100644 --- a/benchmarks/stdlib/crypto/aes_bench.c3 +++ b/benchmarks/stdlib/crypto/aes_bench.c3 @@ -21,11 +21,11 @@ fn void bench_ctr_xcrypt() @benchmark Aes ctx; // encrypt - ctx.init_with_iv(aes, CTR, key, iv); + ctx.init(aes, key, iv); ctx.encrypt_buffer(text, &out); // decrypt - ctx.init_with_iv(aes, CTR, key, iv); + ctx.init(aes, key, iv); ctx.decrypt_buffer(cipher, &out); } diff --git a/lib/std/sort/binarysearch.c3 b/lib/std/sort/binarysearch.c3 index b2e6bb7e2..6bbadfc30 100644 --- a/lib/std/sort/binarysearch.c3 +++ b/lib/std/sort/binarysearch.c3 @@ -3,6 +3,8 @@ module std::sort; <* Perform a binary search over the sorted array and return the index in [0, array.len) where x would be inserted or cmp(i) is true and cmp(j) is true for j in [i, array.len). + + @require @list_is_by_ref(list) : "Expected a list passed by reference or a slice" @require @is_sortable(list) : "The list must be sortable" @require @is_valid_cmp_fn(cmp, list, context) : "Expected a comparison function which compares values" @require @is_valid_context(cmp, context) : "Expected a valid context" @@ -10,42 +12,81 @@ in [0, array.len) where x would be inserted or cmp(i) is true and cmp(j) is true macro usz binarysearch(list, x, cmp = EMPTY_MACRO_SLOT, context = EMPTY_MACRO_SLOT) @builtin { usz i; - usz len = lengthof(list); var $no_cmp = @is_empty_macro_slot(cmp); var $has_context = @is_valid_macro_slot(context); - for (usz j = len; i < j;) - { - usz half = i + (j - i) / 2; - $if $no_cmp: - switch - { - case greater(list[half], x): j = half; - case less(list[half], x): i = half + 1; - default: return half; - } - $else - $switch: - $case $defined(cmp(list[0], list[0], context)): - int res = cmp(list[half], x, context); - $case $defined(cmp(list[0], list[0])): - assert(!$has_context); - int res = cmp(list[half], x); - $case $defined(cmp(&list[0], &list[0], context)): - int res = cmp(&list[half], &x, context); - $case $defined(cmp(&list[0], &list[0])): - assert(!$has_context); - int res = cmp(&list[half], &x); - $default: - assert(false, "Invalid comparison function"); - $endswitch - switch - { - case res > 0: j = half; - case res < 0: i = half + 1; - default: return half; - } - $endif - } - return i; + $if $kindof(list) == SLICE: + usz len = lengthof(list); + for (usz j = len; i < j;) + { + usz half = i + (j - i) / 2; + $if $no_cmp: + switch + { + case greater(list[half], x): j = half; + case less(list[half], x): i = half + 1; + default: return half; + } + $else + + $switch: + $case $defined(cmp(list[0], list[0], context)): + int res = cmp(list[half], x, context); + $case $defined(cmp(list[0], list[0])): + assert(!$has_context); + int res = cmp(list[half], x); + $case $defined(cmp(&list[0], &list[0], context)): + int res = cmp(&list[half], &x, context); + $case $defined(cmp(&list[0], &list[0])): + assert(!$has_context); + int res = cmp(&list[half], &x); + $default: + assert(false, "Invalid comparison function"); + $endswitch + switch + { + case res > 0: j = half; + case res < 0: i = half + 1; + default: return half; + } + $endif + } + $else + usz len = lengthof(*list); + for (usz j = len; i < j;) + { + usz half = i + (j - i) / 2; + $if $no_cmp: + switch + { + case greater((*list)[half], x): j = half; + case less((*list)[half], x): i = half + 1; + default: return half; + } + $else + + $switch: + $case $defined(cmp((*list)[0], (*list)[0], context)): + int res = cmp(list[half], x, context); + $case $defined(cmp((*list)[0], (*list)[0])): + assert(!$has_context); + int res = cmp((*list)[half], x); + $case $defined(cmp(&(*list)[0], &(*list)[0], context)): + int res = cmp(&(*list)[half], &x, context); + $case $defined(cmp(&(*list)[0], &(*list)[0])): + assert(!$has_context); + int res = cmp(&(*list)[half], &x); + $default: + assert(false, "Invalid comparison function"); + $endswitch + switch + { + case res > 0: j = half; + case res < 0: i = half + 1; + default: return half; + } + $endif + } + $endif + return i; } \ No newline at end of file diff --git a/lib/std/sort/countingsort.c3 b/lib/std/sort/countingsort.c3 index f21e1d8dd..08804144d 100644 --- a/lib/std/sort/countingsort.c3 +++ b/lib/std/sort/countingsort.c3 @@ -4,24 +4,39 @@ import std::sort::cs; import std::sort::qs; <* -Sort list using the counting sort algorithm. + + Sort list using the counting sort algorithm. + + @require @list_is_by_ref(list) : "Expected the list to be passed by reference" @require @is_sortable(list) : "The list must be indexable and support .len or .len()" @require @is_cmp_key_fn(key_fn, list) : "Expected a transformation function which returns an unsigned integer." *> macro void countingsort(list, key_fn = EMPTY_MACRO_SLOT) @builtin { - usz len = lengthof(list); - cs::csort{$typeof(list), $typeof(key_fn)}(list, 0, len, key_fn, ~((uint)0)); + $if $kindof(list) == SLICE: + cs::csort{$typeof(list), $typeof(key_fn)}(list, 0, list.len, key_fn, ~((uint)0)); + $else + cs::csort{$typeof(*list), $typeof(key_fn)}(list, 0, lengthof(*list), key_fn, ~((uint)0)); + $endif } macro void insertionsort_indexed(list, start, end, cmp = EMPTY_MACRO_SLOT, context = EMPTY_MACRO_SLOT) @builtin { - is::isort{$typeof(list), $typeof(cmp), $typeof(context)}(list, (usz)start, (usz)end, cmp, context); + $if $kindof(list) == SLICE: + is::isort{$typeof((list)), $typeof(cmp), $typeof(context)}(list, (usz)start, (usz)end, cmp, context); + $else + is::isort{$typeof((*list)), $typeof(cmp), $typeof(context)}(list, (usz)start, (usz)end, cmp, context); + $endif + } macro void quicksort_indexed(list, start, end, cmp = EMPTY_MACRO_SLOT, context = EMPTY_MACRO_SLOT) @builtin { - qs::qsort{$typeof(list), $typeof(cmp), $typeof(context)}(list, (isz)start, (isz)(end-1), cmp, context); + $if $kindof(list) == SLICE: + qs::qsort{$typeof((list)), $typeof(cmp), $typeof(context)}(list, (isz)start, (isz)(end-1), cmp, context); + $else + qs::qsort{$typeof((*list)), $typeof(cmp), $typeof(context)}(list, (isz)start, (isz)(end-1), cmp, context); + $endif } module std::sort::cs{Type, KeyFn}; @@ -42,7 +57,14 @@ alias CmpCallback @if(!KEY_BY_VALUE && NO_KEY_FN) = fn int(ElementType*, Element alias CmpCallback @if(KEY_BY_VALUE && !NO_KEY_FN) = fn int(ElementType, ElementType, KeyFn); alias CmpCallback @if(!KEY_BY_VALUE && !NO_KEY_FN) = fn int(ElementType*, ElementType*, KeyFn); -fn void csort(Type list, usz low, usz high, KeyFn key_fn, uint byte_idx) +const bool IS_SLICE = Type.kindof == SLICE; +alias ListType = $typefrom(IS_SLICE ??? Type : Type*); +macro list_get(ListType l, i) @if(!IS_SLICE) => (*l)[i]; +macro list_get(ListType l, i) @if(IS_SLICE) => l[i]; +macro list_get_ref(ListType l, i) @if(!IS_SLICE) => &(*l)[i]; +macro list_get_ref(ListType l, i) @if(IS_SLICE) => &l[i]; + +fn void csort(ListType list, usz low, usz high, KeyFn key_fn, uint byte_idx) { if (high <= low) return; $if NO_KEY_FN: @@ -67,13 +89,13 @@ fn void csort(Type list, usz low, usz high, KeyFn key_fn, uint byte_idx) { $switch: $case NO_KEY_FN: - KeyFnReturnType k = list[i]; + KeyFnReturnType k = list_get(list, i); $case KEY_BY_VALUE: - KeyFnReturnType k = key_fn(list[i]); + KeyFnReturnType k = key_fn(list_get(list, i)); $case LIST_HAS_REF: - KeyFnReturnType k = key_fn(&list[i]); + KeyFnReturnType k = key_fn(list_get_ref(list, i)); $default: - KeyFnReturnType k = key_fn(&&list[i]); + KeyFnReturnType k = key_fn(&&list_get(list, i)); $endswitch; char key_byte = (char)((k >> (byte_idx * 8)) & 0xff); @@ -131,17 +153,21 @@ fn void csort(Type list, usz low, usz high, KeyFn key_fn, uint byte_idx) { $switch: $case NO_KEY_FN: - KeyFnReturnType k = list[low + s]; + KeyFnReturnType k = list_get(list, low + s); $case KEY_BY_VALUE: - KeyFnReturnType k = key_fn(list[low + s]); + KeyFnReturnType k = key_fn(list_get(list, low + s)); $case LIST_HAS_REF: - KeyFnReturnType k = key_fn(&list[low + s]); + KeyFnReturnType k = key_fn(list_get_ref(list, low + s)); $default: - KeyFnReturnType k = key_fn(&&list[low + s]); + KeyFnReturnType k = key_fn(&&list_get(list, low + s)); $endswitch; char k_idx = (char)(k >> (byte_idx * 8)); usz target_idx = counts[k_idx]; - @swap(list[low + s], list[low + target_idx]); + $if IS_SLICE: + @swap(list[low + s], list[low + target_idx]); + $else + @swap((*list)[low + s], (*list)[low + target_idx]); + $endif counts[k_idx]++; } } diff --git a/lib/std/sort/insertionsort.c3 b/lib/std/sort/insertionsort.c3 index 56771d623..9c5f71721 100644 --- a/lib/std/sort/insertionsort.c3 +++ b/lib/std/sort/insertionsort.c3 @@ -4,38 +4,41 @@ import std::sort::is; <* Sort list using the quick sort algorithm. + @require @list_is_by_ref(list) : "Expected a list passed by reference, or slice passed by value" @require @is_sortable(list) : "The list must be indexable and support .len or .len()" @require @is_valid_cmp_fn(cmp, list, context) : "Expected a comparison function which compares values" *> macro void insertionsort(list, cmp = EMPTY_MACRO_SLOT, context = EMPTY_MACRO_SLOT) @builtin @safemacro { - $if $kindof(list) == POINTER &&& ($kindof(*list) == ARRAY || $kindof(*list) == VECTOR): - $typeof((*list)[0])[] list2 = list; - is::isort{$typeof(list2), $typeof(cmp), $typeof(context)}(list2, 0, list.len, cmp, context); + $if $kindof(list) == SLICE: + is::isort{$typeof(list), $typeof(cmp), $typeof(context)}(list, 0, lengthof(list), cmp, context); $else - usz len = lengthof(list); - is::isort{$typeof(list), $typeof(cmp), $typeof(context)}(list, 0, (isz)len, cmp, context); + is::isort{$typeof(*list), $typeof(cmp), $typeof(context)}(list, 0, lengthof(*list), cmp, context); $endif } module std::sort::is{Type, CmpFn, Context}; alias ElementType = $typeof(((Type){})[0]); +const bool IS_SLICE = Type.kindof == SLICE; +alias ListType = $typefrom(IS_SLICE ??? Type : Type*); +macro ElementType list_get(ListType l, i) => IS_SLICE ??? l[i] : (*l)[i]; +macro ElementType* list_get_ref(ListType l, i) => IS_SLICE ??? &l[i] : &(*l)[i]; -fn void isort(Type list, usz low, usz high, CmpFn comp, Context context) +fn void isort(ListType list, usz low, usz high, CmpFn comp, Context context) { var $has_cmp = @is_valid_macro_slot(comp); var $has_context = @is_valid_macro_slot(context); - var $cmp_by_value = $has_cmp &&& $defined($typefrom(CmpFn.paramsof[0].type) p = list[0]); - var $has_get_ref = $defined(&list[0]); + var $cmp_by_value = $has_cmp &&& $defined($typefrom(CmpFn.paramsof[0].type) p = list_get(list, 0)); + var $has_get_ref = IS_SLICE ||| $defined(&(*list)[0]); for (usz i = low; i < high; ++i) { usz j = i; for (;j > low;) { $if $has_get_ref: - ElementType *rhs = &list[j]; - ElementType *lhs = &list[--j]; + ElementType *rhs = list_get_ref(list, j); + ElementType *lhs = list_get_ref(list, --j); $switch: $case $cmp_by_value && $has_context: if (comp(*rhs, *lhs, context) >= 0) break; @@ -54,17 +57,21 @@ fn void isort(Type list, usz low, usz high, CmpFn comp, Context context) --j; $switch: $case $cmp_by_value && $has_context: - if (comp(list[r], list[j], context) >= 0) break; + if (comp(list_get(list, r), list_get(j), context) >= 0) break; $case $cmp_by_value: - if (comp(list[r], list[j]) >= 0) break; + if (comp(list_get(list, r), list_get(j)) >= 0) break; $case $has_cmp && $has_context: - if (comp(&list[r], &list[j], context) >= 0) break; + if (comp(list_get_ref(list, r), list_get_ref(j), context) >= 0) break; $case $has_cmp: - if (comp(&list[r], &list[j]) >= 0) break; + if (comp(list_get_ref(list, r), list_get_ref(j)) >= 0) break; $default: - if (!less(list[r], list[j])) break; + if (!less(list_get(list, r), list_get(j))) break; $endswitch - @swap(list[r], list[j]); + $if IS_SLICE: + @swap(list[r], list[j]); + $else + @swap((*list)[r], (*list)[j]); + $endif $endif } } diff --git a/lib/std/sort/quicksort.c3 b/lib/std/sort/quicksort.c3 index 22eff4991..fc2d7c638 100644 --- a/lib/std/sort/quicksort.c3 +++ b/lib/std/sort/quicksort.c3 @@ -4,39 +4,50 @@ import std::sort::qs; <* Sort list using the quick sort algorithm. + @require @list_is_by_ref(list) : "Expected a list passed by reference or be a slice" @require @is_sortable(list) : "The list must be indexable and support .len or .len()" @require @is_valid_cmp_fn(cmp, list, context) : "Expected a comparison function which compares values" @require @is_valid_context(cmp, context) : "Expected a valid context" *> macro void quicksort(list, cmp = EMPTY_MACRO_SLOT, context = EMPTY_MACRO_SLOT) @builtin { - $if $kindof(list) == POINTER &&& ($kindof(*list) == ARRAY || $kindof(*list) == VECTOR): - $typeof((*list)[0])[] list2 = list; - qs::qsort{$typeof(list2), $typeof(cmp), $typeof(context)}(list2, 0, (isz)list.len - 1, cmp, context); + $if $kindof(list) == SLICE: + qs::qsort{$typeof(list), $typeof(cmp), $typeof(context)}(list, 0, (isz)list.len - 1, cmp, context); $else - usz len = lengthof(list); - qs::qsort{$typeof(list), $typeof(cmp), $typeof(context)}(list, 0, (isz)len - 1, cmp, context); + qs::qsort{$typeof(*list), $typeof(cmp), $typeof(context)}(list, 0, (isz)lengthof(*list) - 1, cmp, context); $endif } <* -Select the (k+1)th smallest element in an unordered list using Hoare's -selection algorithm (Quickselect). k should be between 0 and len-1. The data -list will be partially sorted. + Select the (k+1)th smallest element in an unordered list using Hoare's + selection algorithm (Quickselect). k should be between 0 and len-1. The data + list will be partially sorted. + @require @list_is_by_ref(list) : "Expected a list passed by reference or be a slice" @require @is_sortable(list) : "The list must be indexable and support .len or .len()" @require @is_valid_cmp_fn(cmp, list, context) : "expected a comparison function which compares values" @require @is_valid_context(cmp, context) : "Expected a valid context" *> macro quickselect(list, isz k, cmp = EMPTY_MACRO_SLOT, context = EMPTY_MACRO_SLOT) @builtin { - usz len = lengthof(list); - return qs::qselect{$typeof(list), $typeof(cmp), $typeof(context)}(list, 0, (isz)len - 1, k, cmp, context); + $if $kindof(list) == SLICE: + return qs::qselect{$typeof(list), $typeof(cmp), $typeof(context)}(list, 0, (isz)list.len - 1, k, cmp, context); + $else + return qs::qselect{$typeof(*list), $typeof(cmp), $typeof(context)}(list, 0, (isz)lengthof(*list) - 1, k, cmp, context); + $endif } module std::sort::qs{Type, CmpFn, Context}; alias ElementType = $typeof(((Type){})[0]); +const bool IS_SLICE = Type.kindof == SLICE; +alias ListType = $typefrom(IS_SLICE ??? Type : Type*); +macro list_get(ListType l, i) @if(!IS_SLICE) => (*l)[i]; +macro list_get(ListType l, i) @if(IS_SLICE) => l[i]; +macro list_get_ref(ListType l, i) @if(!IS_SLICE) => &(*l)[i]; +macro list_get_ref(ListType l, i) @if(IS_SLICE) => &l[i]; +macro list_set(ListType l, i, v) @if(!IS_SLICE) => (*l)[i] = v; +macro list_set(ListType l, i, v) @if(IS_SLICE) => l[i] = v; struct StackElementItem @private { @@ -48,7 +59,7 @@ alias Stack @private = StackElementItem[64]; // Based on https://alienryderflex.com/quicksort by Darel Rex Finley, Public Domain. -fn void qsort(Type list, isz low, isz high, CmpFn cmp, Context context) +fn void qsort(ListType list, isz low, isz high, CmpFn cmp, Context context) { if (low >= 0 && high >= 0 && low < high) { @@ -86,7 +97,7 @@ fn void qsort(Type list, isz low, isz high, CmpFn cmp, Context context) @require low <= k : "kth smallest element is smaller than lower bounds" @require k <= high : "kth smallest element is larger than upper bounds" *> -fn ElementType? qselect(Type list, isz low, isz high, isz k, CmpFn cmp, Context context) +fn ElementType? qselect(ListType list, isz low, isz high, isz k, CmpFn cmp, Context context) { if (low >= 0 && high >= 0 && low < high) { @@ -98,7 +109,7 @@ fn ElementType? qselect(Type list, isz low, isz high, isz k, CmpFn cmp, Context while (l <= h && max_retries--) { pivot = @partition(list, l, h, cmp, context); - if (k == pivot) return list[k]; + if (k == pivot) return list_get(list, k); if (k < pivot) { h = pivot - 1; @@ -112,40 +123,40 @@ fn ElementType? qselect(Type list, isz low, isz high, isz k, CmpFn cmp, Context return NOT_FOUND?; } -macro isz @partition(Type list, isz l, isz h, CmpFn cmp, Context context) +macro isz @partition(ListType list, isz l, isz h, CmpFn cmp, Context context) { var $has_cmp = @is_valid_macro_slot(cmp); var $has_context = @is_valid_macro_slot(context); - var $cmp_by_value = $has_cmp &&& $defined($typefrom(CmpFn.paramsof[0].type) v = list[0]); + var $cmp_by_value = $has_cmp &&& $defined($typefrom(CmpFn.paramsof[0].type) v = list_get(list, 0)); - ElementType pivot = list[l]; + ElementType pivot = list_get(list, l); while (l < h) { $switch: $case $cmp_by_value && $has_context: - while (cmp(list[h], pivot, context) >= 0 && l < h) h--; - if (l < h) list[l++] = list[h]; - while (cmp(list[l], pivot, context) <= 0 && l < h) l++; + while (cmp(list_get(list, h), pivot, context) >= 0 && l < h) h--; + if (l < h) list_set(list, l++, list_get(list, h)); + while (cmp(list_get(list, l), pivot, context) <= 0 && l < h) l++; $case $cmp_by_value: - while (cmp(list[h], pivot) >= 0 && l < h) h--; - if (l < h) list[l++] = list[h]; - while (cmp(list[l], pivot) <= 0 && l < h) l++; + while (cmp(list_get(list, h), pivot) >= 0 && l < h) h--; + if (l < h) list_set(list, l++, list_get(list, h)); + while (cmp(list_get(list, l), pivot) <= 0 && l < h) l++; $case $has_cmp && $has_context: - while (cmp(&list[h], &pivot, context) >= 0 && l < h) h--; - if (l < h) list[l++] = list[h]; - while (cmp(&list[l], &pivot, context) <= 0 && l < h) l++; + while (cmp(list_get_ref(list, h), &pivot, context) >= 0 && l < h) h--; + if (l < h) list_set(list, l++, list_get(list, h)); + while (cmp(list_get_ref(list, l), &pivot, context) <= 0 && l < h) l++; $case $has_cmp: - while (cmp(&list[h], &pivot) >= 0 && l < h) h--; - if (l < h) list[l++] = list[h]; - while (cmp(&list[l], &pivot) <= 0 && l < h) l++; + while (cmp(list_get_ref(list, h), &pivot) >= 0 && l < h) h--; + if (l < h) list_set(list, l++, list_get(list, h)); + while (cmp(list_get_ref(list, l), &pivot) <= 0 && l < h) l++; $default: - while (greater_eq(list[h], pivot) && l < h) h--; - if (l < h) list[l++] = list[h]; - while (less_eq(list[l], pivot) && l < h) l++; + while (greater_eq(list_get(list, h), pivot) && l < h) h--; + if (l < h) list_set(list, l++, list_get(list, h)); + while (less_eq(list_get(list, l), pivot) && l < h) l++; $endswitch - if (l < h) list[h--] = list[l]; + if (l < h) list_set(list, h--, list_get(list, l)); } - list[l] = pivot; + list_set(list, l, pivot); return l; } diff --git a/lib/std/sort/sort.c3 b/lib/std/sort/sort.c3 index 7ebfa4c5d..fc06ed7f3 100644 --- a/lib/std/sort/sort.c3 +++ b/lib/std/sort/sort.c3 @@ -1,50 +1,132 @@ module std::sort; +macro bool @list_is_by_ref(#list) @const +{ + return $kindof(#list) == SLICE ||| ($kindof(#list) == POINTER &&& $kindof(*#list) != SLICE); +} -macro bool @is_sortable(#list) +<* + @require @list_is_by_ref(#list) : "Expected the list to be passed by ref or be a slice" +*> +macro bool @is_sortable(#list) @const { $switch: - $case !$defined(#list[0]): + $case $kindof(#list) == SLICE: + return true; + $case !$defined(*#list): return false; - $case !$defined(#list.len): + $case !$defined((*#list)[0]): return false; - $case $kindof(#list) == VECTOR || $kindof(#list) == ARRAY: + $case !$defined(lengthof(*#list)): return false; - $case $defined(&#list[0]) &&& !types::is_same($typeof(&#list[0]), $typeof(#list[0])*): + $case $defined(&(*#list)[0]) &&& $typeof(&(*#list)[0]) != $typeof((*#list)[0])*: return false; $default: return true; $endswitch } +macro bool @is_any_sortable(#list) @const +{ + $switch $kindof(#list): + $case SLICE: + return true; + $case POINTER: + return @is_sortable(#list); + $default: + return $defined(#list[0]) &&& $defined(lengthof(#list)) + &&& !($defined(&#list[0]) &&& $typeof(&#list[0]) != $typeof(#list[0])*); + $endswitch +} + macro bool @is_valid_context(#cmp, #context) { return @is_valid_macro_slot(#cmp) || @is_empty_macro_slot(#context); } -macro bool @is_valid_cmp_fn(#cmp, #list, #context) + +<* + @require @list_is_by_ref(#list) : "Expected the list to be passed by ref or be a slice" +*> +macro bool @is_valid_cmp_fn(#cmp, #list, #context) @const { var $Type = $typeof(#cmp); var $no_context = @is_empty_macro_slot(#context); $switch: $case @is_empty_macro_slot(#cmp): return true; $case $Type.kindof != FUNC ||| $Type.returns.kindof != SIGNED_INT: return false; - $case $defined(#cmp(#list[0], #list[0], #context)): return true; - $case $defined(#cmp(#list[0], #list[0])): return $no_context; - $case $defined(#cmp(&#list[0], &#list[0], #context)): return true; - $case $defined(#cmp(&#list[0], &#list[0])): return $no_context; - $default: return false; + $default: + $if $kindof(#list) == SLICE: + $switch: + $case $defined(#cmp((#list)[0], (#list)[0], #context)): return true; + $case $defined(#cmp((#list)[0], (#list)[0])): return $no_context; + $case $defined(#cmp(&(#list)[0], &(#list)[0], #context)): return true; + $case $defined(#cmp(&(#list)[0], &(#list)[0])): return $no_context; + $default: return false; + $endswitch + $else + $switch: + $case $defined(#cmp((*#list)[0], (*#list)[0], #context)): return true; + $case $defined(#cmp((*#list)[0], (*#list)[0])): return $no_context; + $case $defined(#cmp(&(*#list)[0], &(*#list)[0], #context)): return true; + $case $defined(#cmp(&(*#list)[0], &(*#list)[0])): return $no_context; + $default: return false; + $endswitch + $endif $endswitch } -macro bool @is_cmp_key_fn(#key_fn, #list) +macro bool @is_any_valid_cmp_fn(#cmp, #list, #context) @const +{ + var $Type = $typeof(#cmp); + var $no_context = @is_empty_macro_slot(#context); + $switch: + $case @is_empty_macro_slot(#cmp): return true; + $case $Type.kindof != FUNC ||| $Type.returns.kindof != SIGNED_INT: return false; + $default: + $if $kindof(#list) != POINTER: + $switch: + $case $defined(#cmp((#list)[0], (#list)[0], #context)): return true; + $case $defined(#cmp((#list)[0], (#list)[0])): return $no_context; + $case $defined(#cmp(&(#list)[0], &(#list)[0], #context)): return true; + $case $defined(#cmp(&(#list)[0], &(#list)[0])): return $no_context; + $default: return false; + $endswitch + $else + $switch: + $case $defined(#cmp((*#list)[0], (*#list)[0], #context)): return true; + $case $defined(#cmp((*#list)[0], (*#list)[0])): return $no_context; + $case $defined(#cmp(&(*#list)[0], &(*#list)[0], #context)): return true; + $case $defined(#cmp(&(*#list)[0], &(*#list)[0])): return $no_context; + $default: return false; + $endswitch + $endif + $endswitch +} + +<* + @require @list_is_by_ref(#list) : "Expected the list to be passed by ref or be a slice" +*> +macro bool @is_cmp_key_fn(#key_fn, #list) @const { $switch: $case @is_empty_macro_slot(#key_fn): return true; $case $kindof(#key_fn) != FUNC: return false; $case $typeof(#key_fn).returns.kindof != UNSIGNED_INT: return false; - $case $defined(#key_fn(#list[0])): return true; - $case $defined(#key_fn(&&(#list[0]))): return true; - $default: return false; + $default: + $if $kindof(#list) == SLICE: + $switch: + $case $defined(#key_fn((#list)[0])): return true; + $case $defined(#key_fn(&&((#list)[0]))): return true; + $default: return false; + $endswitch + $else + $switch: + $case $defined(#key_fn((*#list)[0])): return true; + $case $defined(#key_fn(&&((*#list)[0]))): return true; + $default: return false; + $endswitch + $endif $endswitch + } \ No newline at end of file diff --git a/lib/std/sort/sorted.c3 b/lib/std/sort/sorted.c3 index fa3cffea1..dc6cc653a 100644 --- a/lib/std/sort/sorted.c3 +++ b/lib/std/sort/sorted.c3 @@ -1,13 +1,15 @@ module std::sort; <* -Returns true if list is sorted in either ascending or descending order. - @require @is_sortable(list) : "The list must be indexable and support .len or .len()" - @require @is_valid_cmp_fn(cmp, list, ctx) : "Expected a comparison function which compares values" + Returns true if list is sorted in either ascending or descending order. + + @require @is_any_sortable(list) : "The list must be indexable and support .len or .len()" + @require @is_any_valid_cmp_fn(cmp, list, ctx) : "Expected a comparison function which compares values" @require @is_valid_context(cmp, ctx) : "Expected a valid context" *> macro bool is_sorted(list, cmp = EMPTY_MACRO_SLOT, ctx = EMPTY_MACRO_SLOT) @builtin { + $if ($kindof(list) != POINTER): var $Type = $typeof(list); usz len = lengthof(list); if (len <= 1) return true; @@ -16,6 +18,33 @@ macro bool is_sorted(list, cmp = EMPTY_MACRO_SLOT, ctx = EMPTY_MACRO_SLOT) @buil usz i; int sort_order; + // determine sort order (ascending or descending) + for (i = start; i < end && sort_order == 0; i++) + { + sort_order = @sort_cmp_slice(list, i, cmp, ctx); + } + + // no sort order found, all elements are the same, consider list sorted + if (sort_order == 0) return true; + + // compare adjacent elements to the sort order + for (; i < end; i++) + { + if (sort_order * @sort_cmp_slice(list, i, cmp, ctx) < 0) return false; + } + return true; + }; + + return check_sort(list, 0, len - 1, cmp, ctx); + $else + var $Type = $typeof(*list); + usz len = lengthof(*list); + if (len <= 1) return true; + var check_sort = fn bool($Type* list, usz start, usz end, $typeof(cmp) cmp, $typeof(ctx) ctx) + { + usz i; + int sort_order; + // determine sort order (ascending or descending) for (i = start; i < end && sort_order == 0; i++) { @@ -33,9 +62,33 @@ macro bool is_sorted(list, cmp = EMPTY_MACRO_SLOT, ctx = EMPTY_MACRO_SLOT) @buil return true; }; return check_sort(list, 0, len - 1, cmp, ctx); + $endif } macro int @sort_cmp(list, pos, cmp, ctx) @local +{ + var $has_cmp = @is_valid_macro_slot(cmp); + var $has_context = @is_valid_macro_slot(ctx); + var $cmp_by_value = $has_cmp &&& $defined($typefrom($typeof(cmp).paramsof[0].type) v = (*list)[0]); + + var a = (*list)[pos]; + var b = (*list)[pos+1]; + + $switch: + $case $cmp_by_value && $has_context: + return cmp(a, b); + $case $cmp_by_value: + return cmp(a, b); + $case $has_cmp && $has_context: + return cmp(&a, &b, ctx); + $case $has_cmp: + return cmp(&a, &b); + $default: + return compare_to(a,b); + $endswitch +} + +macro int @sort_cmp_slice(list, pos, cmp, ctx) @local { var $has_cmp = @is_valid_macro_slot(cmp); var $has_context = @is_valid_macro_slot(ctx); diff --git a/releasenotes.md b/releasenotes.md index b28bedccf..5cf36339a 100644 --- a/releasenotes.md +++ b/releasenotes.md @@ -15,6 +15,7 @@ - Incorrect visibility on local globals with public aliases. #2519 ### Stdlib changes +- Sorting functions correctly took slices by value, but also other types by value. Now, only slices are accepted by value, other containers are always by ref. ## 0.7.6 Change list diff --git a/src/compiler/sema_expr.c b/src/compiler/sema_expr.c index 63ccfb26d..43b02ff5b 100644 --- a/src/compiler/sema_expr.c +++ b/src/compiler/sema_expr.c @@ -10693,7 +10693,7 @@ static bool sema_expr_analyse_lenof(SemaContext *context, Expr *expr, bool *miss *missing_ref = true; return false; } - RETURN_SEMA_ERROR(inner, "%s does support lengthof()", type_quoted_error_string(inner->type)); + RETURN_SEMA_ERROR(inner, "%s does not support lengthof()", type_quoted_error_string(inner->type)); } } diff --git a/test/unit/stdlib/collections/map.c3 b/test/unit/stdlib/collections/map.c3 index 91d291976..ab80d2e8c 100644 --- a/test/unit/stdlib/collections/map.c3 +++ b/test/unit/stdlib/collections/map.c3 @@ -47,7 +47,7 @@ fn void map() list.push({key, value}); }; assert(list.len() == tcases.len); - quicksort(list, fn int (MapTest a, MapTest b) => (int)(a.value - b.value)); + quicksort(&list, fn int (MapTest a, MapTest b) => (int)(a.value - b.value)); foreach (i, tc : tcases) { assert(tc.key == list[i].key); diff --git a/test/unit/stdlib/sort/countingsort.c3 b/test/unit/stdlib/sort/countingsort.c3 index 399090266..026cea20b 100644 --- a/test/unit/stdlib/sort/countingsort.c3 +++ b/test/unit/stdlib/sort/countingsort.c3 @@ -79,7 +79,7 @@ fn void countingsort_list() { CountingSortTestList list; list.push_all({ 2, 1, 3}); - sort::countingsort(list, &sort::key_int_value); + sort::countingsort(&list, &sort::key_int_value); assert(check::int_ascending_sort(list.array_view())); } @@ -94,7 +94,7 @@ fn void countingsort_random_large_list() list.push(random.next_int()); } - sort::countingsort(list, &sort::key_int_value); + sort::countingsort(&list, &sort::key_int_value); assert(check::int_ascending_sort(list.array_view())); list.free(); } \ No newline at end of file diff --git a/test/unit/stdlib/sort/insertionsort.c3 b/test/unit/stdlib/sort/insertionsort.c3 index 31d4b1aff..9c044c163 100644 --- a/test/unit/stdlib/sort/insertionsort.c3 +++ b/test/unit/stdlib/sort/insertionsort.c3 @@ -84,7 +84,7 @@ fn void insertionsort_list() { InsertionSortTestList list; list.push_all({ 2, 1, 3}); - sort::insertionsort(list, &sort::cmp_int_value); + sort::insertionsort(&list, &sort::cmp_int_value); assert(check::int_ascending_sort(list.array_view())); } diff --git a/test/unit/stdlib/sort/quicksort.c3 b/test/unit/stdlib/sort/quicksort.c3 index edb81f949..c7eee279d 100644 --- a/test/unit/stdlib/sort/quicksort.c3 +++ b/test/unit/stdlib/sort/quicksort.c3 @@ -84,7 +84,7 @@ fn void quicksort_list() { List2 list; list.push_all({ 2, 1, 3}); - sort::quicksort(list, &sort::cmp_int_value); + sort::quicksort(&list, &sort::cmp_int_value); assert(check::int_sort(list.array_view())); }