Skip to content

Commit

Permalink
Quicksort and insertsort incorrectly allowing arrays and vectors by v…
Browse files Browse the repository at this point in the history
…alue. #1845.
  • Loading branch information
lerno committed Jan 15, 2025
1 parent 29a6a0d commit 2f7d18b
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 12 deletions.
11 changes: 8 additions & 3 deletions lib/std/sort/insertionsort.c3
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@ Sort list using the quick sort algorithm.
@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 insertionsort(list, cmp = EMPTY_MACRO_SLOT, context = EMPTY_MACRO_SLOT) @builtin
macro insertionsort(list, cmp = EMPTY_MACRO_SLOT, context = EMPTY_MACRO_SLOT) @builtin @safemacro
{
usz len = sort::len_from_list(list);
is::isort(<$typeof(list), $typeof(cmp), $typeof(context)>)(list, 0, (isz)len, cmp, context);
$if @typekind(list) == POINTER &&& (@typekind(*list) == ARRAY || @typekind(*list) == VECTOR):
$typeof((*list)[0])[] list2 = list;
is::isort(<$typeof(list2), $typeof(cmp), $typeof(context)>)(list2, 0, list.len, cmp, context);
$else
usz len = sort::len_from_list(list);
is::isort(<$typeof(list), $typeof(cmp), $typeof(context)>)(list, 0, (isz)len, cmp, context);
$endif
}

module std::sort::is(<Type, CmpFn, Context>);
Expand Down
9 changes: 7 additions & 2 deletions lib/std/sort/quicksort.c3
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ Sort list using the quick sort algorithm.
*>
macro quicksort(list, cmp = EMPTY_MACRO_SLOT, context = EMPTY_MACRO_SLOT) @builtin
{
usz len = sort::len_from_list(list);
qs::qsort(<$typeof(list), $typeof(cmp), $typeof(context)>)(list, 0, (isz)len - 1, cmp, context);
$if @typekind(list) == POINTER &&& (@typekind(*list) == ARRAY || @typekind(*list) == VECTOR):
$typeof((*list)[0])[] list2 = list;
qs::qsort(<$typeof(list2), $typeof(cmp), $typeof(context)>)(list2, 0, (isz)list.len - 1, cmp, context);
$else
usz len = sort::len_from_list(list);
qs::qsort(<$typeof(list), $typeof(cmp), $typeof(context)>)(list, 0, (isz)len - 1, cmp, context);
$endif
}

<*
Expand Down
2 changes: 2 additions & 0 deletions lib/std/sort/sort.c3
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ macro bool @is_sortable(#list)
return false;
$case !$defined(#list.len):
return false;
$case @typekind(#list) == VECTOR || @typekind(#list) == ARRAY:
return false;
$case $defined(&#list[0]) &&& !types::is_same($typeof(&#list[0]), $typeof(#list[0])*):
return false;
$default:
Expand Down
1 change: 1 addition & 0 deletions releasenotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@
- Taking the $typeof of a wildcard optional returns `void!`.
- Fix bug with enums with jump tables #1840.
- Enum associated declarations accidentally allowed declaration in function style. #1841
- Quicksort and insertsort incorrectly allowing arrays and vectors by value. #1845.

### Stdlib changes
- Remove unintended print of `char[]` as String
Expand Down
18 changes: 11 additions & 7 deletions src/compiler/sema_expr.c
Original file line number Diff line number Diff line change
Expand Up @@ -9558,18 +9558,22 @@ static inline bool sema_analyse_expr_dispatch(SemaContext *context, Expr *expr,
case EXPR_TYPECALL:
RETURN_SEMA_ERROR(expr, "Expected '()' after this.");
case EXPR_OTHER_CONTEXT:
{
bool in_no_eval = context->call_env.in_no_eval;
context = expr->expr_other_context.context;
expr_replace(expr, expr->expr_other_context.inner);
if (expr->resolve_status == RESOLVE_DONE) return expr_ok(expr);
ASSERT_SPAN(expr, expr->resolve_status == RESOLVE_NOT_DONE);
expr->resolve_status = RESOLVE_RUNNING;
{
bool in_other = context->call_env.in_other;
context->call_env.in_other = true;
bool success = sema_analyse_expr_dispatch(context, expr, check);
context->call_env.in_other = in_other;
return success;
}
bool in_other = context->call_env.in_other;
bool was_in_no_eval = context->call_env.in_no_eval;
context->call_env.in_other = true;
context->call_env.in_no_eval = in_no_eval;
bool success = sema_analyse_expr_dispatch(context, expr, check);
context->call_env.in_other = in_other;
context->call_env.in_no_eval = was_in_no_eval;
return success;
}
case EXPR_CT_CASTABLE:
return sema_expr_analyse_castable(context, expr);
case EXPR_EMBED:
Expand Down
7 changes: 7 additions & 0 deletions test/unit/stdlib/sort/insertionsort.c3
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ fn void insertionsort_with_value()
}
}

fn void insertionsort_with_array()
{
int[*] a = { 4, 8, 100, 1, 2 };
sort::insertionsort(&a);
assert(a == { 1, 2, 4, 8, 100 });
}

fn void insertionsort_with_lambda()
{
int[][] tcases = {
Expand Down
7 changes: 7 additions & 0 deletions test/unit/stdlib/sort/quicksort.c3
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ fn void quicksort_with_ref()
}
}

fn void quicksort_with_array()
{
int[*] a = { 4, 8, 100, 1, 2 };
sort::quicksort(&a);
assert(a == { 1, 2, 4, 8, 100 });
}

fn void quicksort_with_value()
{
int[][] tcases = {
Expand Down

0 comments on commit 2f7d18b

Please sign in to comment.