diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bf7d0ee..c2a7b725 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,30 @@ All notable changes to Hemlock will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.9.1] - 2026-07-28 + +Patch release. `array.sort()` was quadratic on inputs programs routinely +produce, and crashed outright on large ones in compiled code. + +### Fixed + +- **`array.sort()` no longer degrades to quadratic time, and no longer segfaults on large sorted arrays.** The compiled runtime used a Lomuto-partition quicksort with a last-element pivot, no randomization, recursing on both halves — so sorted or reverse-sorted input hit the worst case in both dimensions at once: O(n²) comparisons *and* O(n) recursion depth. A presorted 200k-element array segfaulted from stack exhaustion, and 2000 elements took ~5 ms instead of ~0 ms. Sorted input is not an exotic case; it is what any incremental workload (re-sorting a mostly-ordered list each frame or each tick) produces by construction. The interpreter used an insertion sort — the mirror image: linear on presorted input, quadratic on shuffled. + + Both backends now use the same **stable bottom-up merge sort**: O(n log n) comparisons on every input with no quadratic worst case, and iterative, so no array size can exhaust the C stack. Each merge first checks whether its two runs are already in order and skips the copy when they are, keeping fully sorted input at O(n) comparisons with no data movement. + + Measured on the compiled backend: presorted 200k now sorts in 1 ms (previously segfaulted); presorted 2000 drops from ~5 ms to ~0 ms. Interpreter: 50k shuffled sorts in 7 ms. + +- **`sort()` is stable on both backends.** Quicksort is unstable and insertion sort is stable, so the two backends disagreed on the order of equal-comparing elements and `sort()` had no usable stability contract. Ties now keep their original relative order everywhere, and that is a documented guarantee rather than a backend-dependent accident. + +- **A comparator that throws leaves the array intact.** Each merge builds its output in scratch space and copies back only once complete, so an abandoned sort leaves the array a valid permutation of its elements — every element still present exactly once (the ordering is unspecified). The old interpreter path returned mid-shift, duplicating one slot and dropping an element, which corrupted reference counts. + +- **A comparator that resizes the array being sorted raises instead of corrupting memory.** Growing the array from inside a comparator reallocates the element storage out from under the in-progress merge. This now raises a catchable `sort() comparator resized the array being sorted` rather than being a use-after-free. Sorting a *different* array from inside a comparator is still fine. + +### Notes + +- Known limitation, unchanged in kind from `map()`/`filter()`: in compiled code a throwing comparator `longjmp`s past the sort and leaks the scratch buffer, since `hml_throw()` neither unwinds nor runs defers and there is no cleanup hook to attach the `free()` to. The array itself stays intact, which is the property that matters for reference counting. +- New `tests/parity/methods/array_sort.hml` covers default and custom comparators, stability, presorted/reversed/shuffled input, edge cases, and the throwing-comparator path. Suites: 332/332 parity, 54 compiler; verified clean under valgrind on both backends. + ## [2.9.0] - 2026-07-27 Semantics-hardening release. The theme is making the documented contracts diff --git a/docs/reference/array-api.md b/docs/reference/array-api.md index c1d5502f..5783bc41 100644 --- a/docs/reference/array-api.md +++ b/docs/reference/array-api.md @@ -811,7 +811,22 @@ words.sort(); print(words); // ["apple", "banana", "cherry"] ``` -**Note:** Default comparison orders by value type, then by value within type. Uses stable insertion sort. +**Note:** Default comparison orders by value type, then by value within type. + +Uses a stable bottom-up merge sort: equal-comparing elements keep their original +relative order, and every input costs O(n log n) comparisons — there is no input +that degrades it to quadratic time. Already-sorted input is detected run by run +and costs O(n) comparisons with no data movement. The sort is iterative, so no +array size can exhaust the C stack. It allocates a scratch buffer of `n` values +for the duration of the sort. + +If the comparator throws, the sort is abandoned and the exception propagates. The +array is left as a valid permutation of its elements — every element is still +present exactly once — but the ordering is unspecified. + +A comparator that resizes the array being sorted (via `push`, `pop`, and so on) +throws `sort() comparator resized the array being sorted`. Sorting a *different* +array from inside a comparator is fine. --- diff --git a/include/version.h b/include/version.h index d10dc8fa..d6f10922 100644 --- a/include/version.h +++ b/include/version.h @@ -10,13 +10,14 @@ #define HEMLOCK_VERSION_MAJOR 2 #define HEMLOCK_VERSION_MINOR 9 -#define HEMLOCK_VERSION_PATCH 0 +#define HEMLOCK_VERSION_PATCH 1 -#define HEMLOCK_VERSION "2.9.0" +#define HEMLOCK_VERSION "2.9.1" #define HEMLOCK_VERSION_STRING "Hemlock v" HEMLOCK_VERSION /* * Version history: + * 2.9.1 - array.sort() is now a stable bottom-up merge sort in both backends, replacing a Lomuto quicksort (compiler) and an insertion sort (interpreter) that were each quadratic on inputs programs routinely produce. The compiled runtime's quicksort took a last-element pivot with no randomization and recursed on both halves, so sorted or reverse-sorted input hit the worst case in both dimensions at once: O(n^2) comparisons AND O(n) recursion depth. A presorted 200k-element array segfaulted from stack exhaustion, and 2000 elements took ~5ms instead of ~0ms — and sorted input is not an exotic case, it is what any incremental workload (re-sorting a mostly-ordered list each frame or each tick) produces by construction. The interpreter's insertion sort was the mirror image: linear on presorted input, quadratic on shuffled. The two backends also disagreed on equal-comparing elements, since quicksort is unstable and insertion sort is stable, so sort() had no usable stability contract. Both now run the same iterative merge sort: O(n log n) comparisons on every input with no quadratic worst case, no recursion so no array size can exhaust the C stack, and stable — which makes tie order a documented guarantee rather than a backend-dependent accident. Each merge first checks whether its two runs are already in order and skips the copy when they are, keeping fully sorted input at O(n) comparisons and zero data movement. Presorted 200k now sorts in 1ms (was: segfault); presorted 2000 drops ~5ms to ~0ms; 50k shuffled sorts in 7ms interpreted. Two correctness properties come with the rewrite: each merge builds its output in scratch space and copies back only once complete, so a comparator that throws leaves the array a valid permutation of its elements instead of a buffer with duplicated slots and corrupted reference counts (the old interpreter path returned mid-shift and lost an element); and a comparator that resizes the array being sorted — which would dangle the element pointer the merge is walking — now raises a catchable error instead of a use-after-free. Known limitation, unchanged in kind from map()/filter(): in compiled code a throwing comparator longjmps past the sort and leaks the scratch buffer, because hml_throw() neither unwinds nor runs defers, so there is no cleanup hook to attach the free() to. New tests/parity/methods/array_sort.hml covers default and custom comparators, stability, presorted/reversed/shuffled input, edge cases, and the throwing-comparator path; verified clean under valgrind on both backends. Suites: 332/332 parity, 54 compiler. * 2.9.0 - Semantics-hardening release: the documented contracts are now actually true on both backends, because each was formalized, audited against the implementation, and fixed where it disagreed. ARITHMETIC: the checked-arithmetic contract (i32/i64 add/sub/mul throw catchable "Integer overflow", narrow and unsigned wrap, `/` always returns float) was full of holes that made the same expression behave differently depending on whether it was written as literals or variables, inside a string concatenation, or run through the compiler. The frontend optimizer no longer folds i32/i64 overflow (2147483647 + 1 folded to an i64 instead of throwing), folds u64 literals in uint64, compares 64-bit ints in integer precision rather than through double, folds shifts with i32 width semantics, and no longer applies semantics-changing rewrites (x/1 returned an int where / must return float; x+0 / x*1 / x|0 changed typeof for narrow operands; !!x and -(-x) leaked non-bool values and skipped negation overflow; x && true returned the operand where && always returns bool). The interpreter's hidden integer-division mode inside string concatenation is gone ("x" + 7/2 gave "x3" interpreted, "x3.5" compiled), INT_MIN % -1 is 0 instead of a process-killing SIGFPE, and negating INT32_MIN/INT64_MIN throws instead of wrapping. Codegen's native fast paths emit overflow-checked code for i32/i64 (raw C signed arithmetic was UB and skipped the throw) and fall back to the runtime path whenever raw C semantics differ from promoted-type semantics; codegen's private constant folder was deleted (it re-folded deliberately-unfolded expressions with the old broken semantics); both build trees and all generated C now compile with -fwrapv so documented wrapping is defined rather than UB. TYPES: variable annotations are enforced on reassignment in both backends — `let x: i32 = 7; x = "hello";` silently rebound x in the interpreter and dynamic values bypassed the compiler's static check entirely, since only initial declarations were ever checked. MEMORY SAFETY: new docs/design/memory-safety-model.md (RustBelt-style invariants B1-B8, safe fragment, memory-safety theorem, per-crossing proof obligations for every ptr operation) whose audit found real counterexamples reachable from buffer-only code, all fixed: free() with a live slice view left the view dangling (roots now carry an atomic view_count and refuse the free), overlapping buffer-to-buffer copies invoked C memcpy on overlapping ranges (now memmove), buffer(0) hard-exited uncatchably in the interpreter while compiled code returned a 0-length buffer, buffers were malloc'd in one backend and calloc'd in the other despite docs promising zeroing, and buffer_ptr() on a freed buffer silently returned NULL. Separately, interpreter bounds checks did not stop the access: runtime_error() does not unwind, so b[-1] = 9 reported the error and still stored to data[-1] (smashing the malloc chunk header) and b[0] after free(b) still dereferenced NULL. CONCURRENCY: new normative docs/advanced/memory-model.md (value taxonomy, six happens-before edges, race-freedom theorem, and the rule that shared bindings are not a communication channel), whose writing exposed two unbuffered-channel rendezvous bugs — a second concurrent sender staged over an already-staged value (dropping a message, leaking its reference), and multi-sender rendezvous detected success as sender_waiting == 0, which a re-staging sender plus a mis-delivered cond_signal turns into a permanently asleep sender; both fixed with a per-channel rendezvous generation counter. CODEGEN: a systematic differential audit found and fixed 30+ interpreter/compiler divergences — concat-chain left-associativity (1 + 2 + "x" compiled to "12x"), unsigned/narrow operands collapsing into signed fast paths (u64 >> u64 went negative), OR-patterns with bindings failing to compile, define-typed patterns matching any object, break inside a loop nested in a switch exiting the switch, eager instead of lazy case-value evaluation, jumps out of try skipping finally, labeled break/continue skipping unboxed write-back, try-body locals indeterminate after longjmp, rune lets unboxing to i32, named arguments bound positionally by the inliner and dropped by method calls, ref arguments corrupting the stack for unboxed variables and losing mutations for element/property refs, and receiver-type guards for string/file/channel-only methods so user objects with those method names dispatch. Plus: compiled `ptr == null` is now true for a NULL pointer (every FFI null-guard was dead code in compiled builds), finally runs when catch throws, extracted methods keep self bound, nested closures share captured variables by reference instead of a snapshot copy, and inlining is declined when a parameter name collides with an unboxed caller local (which emitted uncompilable C). REFCOUNTING: audit of both backends — ref-param double release (deterministic use-after-free), defer leaking results and clobbering the enclosing return value, VAL_WEBSOCKET missing from refcounting, caught exceptions leaking stack frames until "Maximum call stack depth exceeded", unrefcounted socket handles leaking per accepted connection, hml_throw leaking one reference per throw, s[i] = c mutating the shared ASCII literal pool, and codegen not owning parameters, catch parameters, or closure captures across early exits. TOOLING: new `hemlock check [--json]` subcommand runs every static pass (syntax with multi-error recovery, lint, type check, borrow check) without executing and reports all diagnostics rather than stopping at the first failing pass; the LSP publishes real per-error syntax diagnostics with the parser's own messages instead of one generic marker; docs/reference/stdlib-api-index.md is a generated one-line-per-symbol index of all 1050 stdlib exports (make stdlib-api, freshness-verified by the new make docs-check); `hemlock -e` executes imports (it never routed through the module system); and a parser segfault on import/export with a missing module path is fixed in every consumer. Suites: 765 interpreter, compiler, 331/331 parity, 23 contracts, 8 check, docs-check. * 2.8.2 - Compiler-diagnostics and runtime-correctness batch. RUNTIME: fixed a data race in the single-char ASCII string pool. init_ascii_strings() guarded a 128-entry table fill with a plain `static int initialized` and is called from hml_val_string() on the hot path for every single-character string, so two threads could each observe 0 and both run the fill (double-allocating all 128 immortal entries and orphaning one set), and because `initialized = 1` was written AFTER the fill with no release barrier a third thread could observe the flag set and read a not-yet-published entry — a garbage HmlString*, i.e. use of uninitialized memory, not merely a sanitizer complaint. ThreadSanitizer flagged it at src/value.c:253 under the concurrent_object_read stress test; it is timing-dependent, so the lane was usually green. Fixed by deleting the runtime initialization rather than synchronizing it: the pool is now a statically-initialized HmlString[128] built by macro expansion, each entry's `data` pointing at its own inline_data (an address constant, valid in a static initializer). That makes the race impossible by construction rather than unlikely, and drops 128 startup mallocs plus the per-call init branch from a hot path; the entries were already immortal and never freed, so static storage matches their real lifetime. Second bug found in this pool (2.4.6 fixed the immortal-sentinel countdown) — a fixed table of constants had no business being built at runtime. COMPILER: --strict-types was unusable on any multi-file project — running it over a ~19k-line codebase (Witchgrid) produced 96 "identifier has unknown type" warnings, all spurious, all anchored at line 0. Three causes: (1) type_check_stmt had no STMT_IMPORT case, so imported names fell through to `default: break;` and resolved to nothing — visible only where inference actually runs, so `"" + NAME` in a concat warned while a bare call did not; imports now bind ANY, since modules are type-checked separately and the exported type is not knowable without cross-module inference. (2) expr_ident() defaults line/column to 0 and the parser never stamped them, so every identifier-anchored diagnostic printed "file:0" (statements were unaffected, which is why lint warnings looked correct). (3) a forward reference from a function body to a module-level let/const declared further down the file was reported unknown, though the body only runs after module init; top-level let/const names are now collected before checking — names only, so redeclaration and shadowing checks are unchanged. Same codebase now reports 0, and a genuinely undefined identifier still warns, at its real line. TESTS: compile-failure fixtures declare themselves with `// @expect-compile-fail ` in their first 20 lines instead of being tracked by a hand-kept path list inside run_repo_compile_check.sh — the expectation now lives next to the code encoding it, so adding a negative fixture no longer silently breaks CI until someone edits a shell script. All 20 converted and both directory rules dropped (they covered one file and zero files); classification verified identical before/after across every .hml in the tree. New parity fixture walks all 127 non-NUL ASCII pool entries, which is what catches a bad macro expansion that ordinary tests (touching a handful of characters) would miss. Also includes the previously-unreleased #613/#614 work: interprocedural borrow-checker consume detection + container escape and channel/task/FFI/mmap tracking; a static lint pass (unreachable code, dead branches, self-assign, % 0, self-compare, dup-field, dup-case, infinite loops that never fall through); and rejection of unknown named arguments at compile time — previously the type checker skipped them and only the interpreter threw, while the compiled binary silently bound the argument positionally, so the same program behaved differently on the two backends. Suites: 759 interpreter, 54 compiler, 304/304 parity, 18 lint, stress 11/11 under both tsan and asan. * 2.8.1 - Windows: hash builtins (sha1/sha256/sha512/md5 — and @stdlib/hash on top of them) now work natively, backed by Windows CNG (bcrypt.dll, a system DLL — binaries stay self-contained) instead of throwing HEMLOCK_NO_OPENSSL errors. exec()/exec_argv() also work now (CreateProcess + pipes via hml_win32_run_capture in the platform layer): shell mode through %COMSPEC% /S /C like popen through /bin/sh, argv mode direct with CommandLineToArgvW quoting, captured stdout/stderr, stdin: feeding, and POSIX-mirroring exit-code-127 objects on launch failure — unlocking @stdlib/shell run()/run_capture() on Windows (Unix-command helpers like ls()/which() stay Unix-only). NEW __term_* builtins (is_tty/raw/read_byte/size, src/shared/term_core.c: POSIX termios+poll+TIOCGWINSZ, Windows console API with VT input/output so ESC-sequence parsing and ANSI output work unmodified) put @stdlib/termios raw mode and @stdlib/terminal on Windows; terminal.hml stops shelling out (stty size → __term_size, per-frame __exec("printf ...") → write()). Compiler fix: imports nested in if/else platform gates emitted _ffi_lib_ loads without declaring the global (any program importing @stdlib/termios failed to compile); declaration pass now recurses. No-FFI builds (HEMLOCK_NO_FFI/WASM): import/extern-fn statements warn instead of throwing so platform-gated-FFI modules load. @stdlib/regex works on Windows: musl 1.2.5's TRE regcomp/regexec vendored (BSD-2, src/shared/regex_win32 + regex_compat.c unity TU, include path supplies on MinGW only) — regex builtins' POSIX code path now compiles on every platform, stubs deleted; full regex parity output matches Linux under Wine in both backends (only TRE-vs-glibc error wording differs). posix_spawn/waitpid/kill work on Windows (CreateProcess detached spawn with env/cwd/stdio-fd/setsid options; pid->handle registry = zombie analogue, no pid-reuse race; waitpid blocking/WNOHANG with POSIX-encoded status so status>>8 is portable; kill 0 probes, other signals terminate with 128+sig) — only fork/wait-for-any/uid family remain POSIX-only. stdlib waitpid became a defaulted-options wrapper (bare __waitpid re-export had fixed arity 2 compiled; documented 1-arg form only worked interpreted; parity test added). Gap-hunting tests added (import-all-stdlib, argv-quoting hazards, >64K dual-stream exec output, builtin-method-name shadowing) which surfaced + fixed three compiler bugs that kept @stdlib/mmap interpreter-only forever: let-bound module fns called from object methods emitted nonexistent _modN_fn_* symbols; __mmap_open's function-value num_params/num_required were swapped; compiled method dispatch sent .read_u8/.write_u8/.read_bytes/.write_bytes on ANY receiver to the buffer builtins and discarded .close()'s object-fallback return — now type-guarded with hml_call_method fallback. tests/stdlib_mmap passes identically in both backends. Also NEW: __random_bytes(n) CSPRNG builtin (BCryptGenRandom on Windows, /dev/urandom elsewhere; both backends + parity test) — @stdlib/crypto random_bytes() drops its OpenSSL RAND_bytes dependency, and @stdlib/uuid calls the builtin directly instead of importing @stdlib/crypto, fixing UUID generation on Windows (crypto's module-level libcrypto import can't load there); @stdlib/path expand_user() resolves ~ via __homedir() (USERPROFILE on Windows) instead of $HOME//home/ guessing. Implemented as hml_win32_hash() in src/shared/platform_win32.c (BCryptOpenAlgorithmProvider/CreateHash/HashData/FinishHash, chunked for >4 GB inputs since BCryptHashData takes a ULONG), declared in hemlock_compat.h (the windows.h-free header — the interpreter's internal.h can't include windows.h because winnt.h's TokenType collides with the lexer's), and wired into both backends: interpreter builtins/crypto.c gains an #elif _WIN32 CNG branch (its hash stubs dropped from wasm_interp_shim.c on Windows; ECDSA stubs remain — CNG emits raw r||s signatures where the OpenSSL builtins produce DER, no compatible mapping), runtime builtins_crypto.c gains the same branch for compiled programs. -lbcrypt added to every Windows link: hemlock.exe, hemlockc.exe, runtime LDFLAGS, and both generated hemlockc link commands (static + default). CI smoke tests now assert sha256("hello")/md5("hello") digests in all four lanes (native + Wine, interpreter + compiled). docs/advanced/windows.md crypto limitation row reduced to ECDSA-only. diff --git a/runtime/src/builtins_array.c b/runtime/src/builtins_array.c index d9a76ead..8ea47cfb 100644 --- a/runtime/src/builtins_array.c +++ b/runtime/src/builtins_array.c @@ -766,10 +766,12 @@ HmlValue hml_array_flat(HmlValue arr) { return result; } -// Quicksort comparator context +// Merge sort comparator context typedef struct { HmlValue comparator; int has_comparator; + HmlArray *array; // array being sorted, for the mid-sort resize check + int expected_length; } SortContext; // Default comparison function for sorting @@ -798,7 +800,11 @@ static int default_compare(HmlValue a, HmlValue b) { } } -// Compare two values using either default or custom comparator +// Compare two values using either default or custom comparator. +// +// A comparator that resizes the array mid-sort would invalidate the element +// pointer the merge is walking, so that is rejected with a clear error rather +// than left as a use-after-free. static int compare_values(HmlValue a, HmlValue b, SortContext *ctx) { if (!ctx->has_comparator) { return default_compare(a, b); @@ -809,36 +815,71 @@ static int compare_values(HmlValue a, HmlValue b, SortContext *ctx) { HmlValue result = hml_call_function(ctx->comparator, args, 2); int cmp = hml_to_i32(result); hml_release(&result); + + if (ctx->array->length != ctx->expected_length) { + hml_runtime_error("sort() comparator resized the array being sorted"); + } return cmp; } -// Quicksort partition -static int partition(HmlValue *arr, int low, int high, SortContext *ctx) { - HmlValue pivot = arr[high]; - int i = low - 1; - - for (int j = low; j < high; j++) { - if (compare_values(arr[j], pivot, ctx) <= 0) { - i++; - HmlValue tmp = arr[i]; - arr[i] = arr[j]; - arr[j] = tmp; +// Merge the adjacent sorted runs arr[lo..mid) and arr[mid..hi). +// +// The merged output is built in scratch[lo..hi) and only copied back over the +// array once the merge has completed. A comparator that throws longjmps out of +// this function, so leaving `arr` untouched until the very end is what keeps +// the array a valid permutation of its elements (no duplicated slots, so no +// reference-count corruption) when a sort is abandoned mid-flight. +static void merge_runs(HmlValue *arr, HmlValue *scratch, int lo, int mid, int hi, + SortContext *ctx) { + // Fast path: the two runs are already in order, so nothing has to move. + // This is what keeps an already-sorted array at O(n) comparisons overall. + if (compare_values(arr[mid - 1], arr[mid], ctx) <= 0) { + return; + } + + int i = lo, j = mid, k = lo; + while (i < mid && j < hi) { + // `<= 0` takes from the left run on ties, which makes the sort stable. + if (compare_values(arr[i], arr[j], ctx) <= 0) { + scratch[k++] = arr[i++]; + } else { + scratch[k++] = arr[j++]; } } + while (i < mid) scratch[k++] = arr[i++]; + while (j < hi) scratch[k++] = arr[j++]; - HmlValue tmp = arr[i + 1]; - arr[i + 1] = arr[high]; - arr[high] = tmp; - return i + 1; + memcpy(arr + lo, scratch + lo, (size_t)(hi - lo) * sizeof(HmlValue)); } -// Quicksort implementation -static void quicksort(HmlValue *arr, int low, int high, SortContext *ctx) { - if (low < high) { - int pi = partition(arr, low, high, ctx); - quicksort(arr, low, pi - 1, ctx); - quicksort(arr, pi + 1, high, ctx); +// Bottom-up merge sort: stable, O(n log n) comparisons on every input, and +// iterative so no input can blow the C stack. Replaces a Lomuto quicksort that +// degraded to O(n^2) time and O(n) recursion depth on sorted input. +// +// Note: a comparator that throws longjmps straight past this frame, so the +// scratch buffer is leaked on that path. hml_throw() does not unwind or run +// defers, so there is no cleanup hook to hang the free() on; the same is true +// of the result array in map()/filter(). The array itself stays intact, which +// is the property that matters for reference counting. +static void merge_sort(HmlValue *arr, int n, SortContext *ctx) { + if ((size_t)n > SIZE_MAX / sizeof(HmlValue)) { + hml_runtime_error("sort() array too large"); + } + HmlValue *scratch = malloc((size_t)n * sizeof(HmlValue)); + if (!scratch) { + hml_runtime_error("sort() failed to allocate scratch space for %d elements", n); } + + // `width` is 64-bit so the doubling cannot overflow for large `n`. + for (long long width = 1; width < n; width *= 2) { + for (long long lo = 0; lo + width < n; lo += 2 * width) { + long long end = lo + 2 * width; + merge_runs(arr, scratch, (int)lo, (int)(lo + width), + end < n ? (int)end : n, ctx); + } + } + + free(scratch); } void hml_array_sort(HmlValue arr, HmlValue comparator) { @@ -854,8 +895,10 @@ void hml_array_sort(HmlValue arr, HmlValue comparator) { SortContext ctx; ctx.comparator = comparator; ctx.has_comparator = (comparator.type == HML_VAL_FUNCTION); + ctx.array = a; + ctx.expected_length = a->length; - quicksort(a->elements, 0, a->length - 1, &ctx); + merge_sort(a->elements, a->length, &ctx); } void hml_array_fill(HmlValue arr, HmlValue value, HmlValue start, HmlValue end) { diff --git a/src/backends/interpreter/io/array_methods.c b/src/backends/interpreter/io/array_methods.c index 309f7be3..ac5d8def 100644 --- a/src/backends/interpreter/io/array_methods.c +++ b/src/backends/interpreter/io/array_methods.c @@ -88,6 +88,133 @@ static Value call_function_value(Value func, Value *args, int num_args, Executio return result; } +// ========== ARRAY SORTING ========== + +// Default ordering used by sort() when no comparator is supplied: values are +// ordered by type tag first, then by value within a type. +static int sort_default_compare(Value a, Value b) { + if (a.type != b.type) { + return (int)a.type - (int)b.type; + } + + switch (a.type) { + case VAL_I8: return (a.as.as_i8 < b.as.as_i8) ? -1 : (a.as.as_i8 > b.as.as_i8) ? 1 : 0; + case VAL_I16: return (a.as.as_i16 < b.as.as_i16) ? -1 : (a.as.as_i16 > b.as.as_i16) ? 1 : 0; + case VAL_I32: return (a.as.as_i32 < b.as.as_i32) ? -1 : (a.as.as_i32 > b.as.as_i32) ? 1 : 0; + case VAL_I64: return (a.as.as_i64 < b.as.as_i64) ? -1 : (a.as.as_i64 > b.as.as_i64) ? 1 : 0; + case VAL_U8: return (a.as.as_u8 < b.as.as_u8) ? -1 : (a.as.as_u8 > b.as.as_u8) ? 1 : 0; + case VAL_U16: return (a.as.as_u16 < b.as.as_u16) ? -1 : (a.as.as_u16 > b.as.as_u16) ? 1 : 0; + case VAL_U32: return (a.as.as_u32 < b.as.as_u32) ? -1 : (a.as.as_u32 > b.as.as_u32) ? 1 : 0; + case VAL_U64: return (a.as.as_u64 < b.as.as_u64) ? -1 : (a.as.as_u64 > b.as.as_u64) ? 1 : 0; + case VAL_F32: return (a.as.as_f32 < b.as.as_f32) ? -1 : (a.as.as_f32 > b.as.as_f32) ? 1 : 0; + case VAL_F64: return (a.as.as_f64 < b.as.as_f64) ? -1 : (a.as.as_f64 > b.as.as_f64) ? 1 : 0; + case VAL_BOOL: return (int)a.as.as_bool - (int)b.as.as_bool; + case VAL_STRING: return strcmp(a.as.as_string->data, b.as.as_string->data); + default: return 0; // Objects, arrays, etc. - no default ordering + } +} + +// Compare two values with either the default ordering or a user comparator. +// Writes the result to *out and returns 1; returns 0 if the comparator threw. +// +// A comparator that resizes the array mid-sort would invalidate the element +// pointer the merge is walking, so that is rejected with a clear error rather +// than left as a use-after-free. +static int sort_compare(Value a, Value b, Value comparator, int has_comparator, + Array *arr, int expected_length, ExecutionContext *ctx, int *out) { + if (!has_comparator) { + *out = sort_default_compare(a, b); + return 1; + } + + Value cmp_args[2] = { a, b }; + Value cmp_result = call_function_value(comparator, cmp_args, 2, ctx); + if (ctx->exception_state.is_throwing) { + return 0; + } + *out = value_to_int(cmp_result); + value_release(cmp_result); + + if (arr->length != expected_length) { + throw_runtime_error(ctx, "sort() comparator resized the array being sorted"); + return 0; + } + return 1; +} + +// Merge the adjacent sorted runs arr[lo..mid) and arr[mid..hi). +// +// The merged output is built in scratch[lo..hi) and only copied back over the +// array once the merge has completed, so a comparator that throws part-way +// through leaves the array as a valid permutation of its elements: no slot is +// ever duplicated, so no element's reference count is corrupted. +static int sort_merge_runs(Array *arr, Value *scratch, int lo, int mid, int hi, + Value comparator, int has_comparator, int n, + ExecutionContext *ctx) { + Value *elems = arr->elements; + int cmp; + + // Fast path: the two runs are already in order, so nothing has to move. + // This is what keeps an already-sorted array at O(n) comparisons overall. + if (!sort_compare(elems[mid - 1], elems[mid], comparator, has_comparator, arr, n, ctx, &cmp)) { + return 0; + } + if (cmp <= 0) { + return 1; + } + + int i = lo, j = mid, k = lo; + while (i < mid && j < hi) { + if (!sort_compare(elems[i], elems[j], comparator, has_comparator, arr, n, ctx, &cmp)) { + return 0; + } + // `<= 0` takes from the left run on ties, which makes the sort stable. + if (cmp <= 0) { + scratch[k++] = elems[i++]; + } else { + scratch[k++] = elems[j++]; + } + } + while (i < mid) scratch[k++] = elems[i++]; + while (j < hi) scratch[k++] = elems[j++]; + + memcpy(elems + lo, scratch + lo, (size_t)(hi - lo) * sizeof(Value)); + return 1; +} + +// Sort an array in place with a stable bottom-up merge sort: O(n log n) +// comparisons on every input, and iterative so no input can exhaust the C +// stack. Returns 0 if the sort was abandoned (comparator threw, or scratch +// space could not be allocated), in which case ctx holds the exception. +static int array_merge_sort(Array *arr, Value comparator, int has_comparator, + ExecutionContext *ctx) { + int n = arr->length; + if ((size_t)n > SIZE_MAX / sizeof(Value)) { + throw_runtime_error(ctx, "sort() array too large"); + return 0; + } + Value *scratch = malloc((size_t)n * sizeof(Value)); + if (!scratch) { + throw_runtime_error(ctx, "sort() failed to allocate scratch space for %d elements", n); + return 0; + } + + // `width` is 64-bit so the doubling cannot overflow for large `n`. + for (long long width = 1; width < n; width *= 2) { + for (long long lo = 0; lo + width < n; lo += 2 * width) { + long long end = lo + 2 * width; + if (!sort_merge_runs(arr, scratch, (int)lo, (int)(lo + width), + end < n ? (int)end : n, comparator, has_comparator, n, ctx)) { + free(scratch); + return 0; + } + } + } + + free(scratch); + return 1; +} + // ========== ARRAY METHOD HANDLING ========== // Helper function to check if value matches array element type @@ -248,58 +375,9 @@ Value call_array_method(Array *arr, const char *method, Value *args, int num_arg return val_null(); // Already sorted } - // Simple insertion sort (stable, good for small arrays) - int has_comparator = (num_args == 1); - - for (int i = 1; i < arr->length; i++) { - Value key = arr->elements[i]; - int j = i - 1; - - while (j >= 0) { - int cmp; - if (has_comparator) { - Value cmp_args[2]; - cmp_args[0] = arr->elements[j]; - cmp_args[1] = key; - Value cmp_result = call_function_value(args[0], cmp_args, 2, ctx); - if (ctx->exception_state.is_throwing) { - return val_null(); - } - cmp = value_to_int(cmp_result); - value_release(cmp_result); - } else { - // Default comparison by value type - Value a = arr->elements[j]; - Value b = key; - - // Compare by type first - if (a.type != b.type) { - cmp = (int)a.type - (int)b.type; - } else { - switch (a.type) { - case VAL_I8: cmp = (a.as.as_i8 < b.as.as_i8) ? -1 : (a.as.as_i8 > b.as.as_i8) ? 1 : 0; break; - case VAL_I16: cmp = (a.as.as_i16 < b.as.as_i16) ? -1 : (a.as.as_i16 > b.as.as_i16) ? 1 : 0; break; - case VAL_I32: cmp = (a.as.as_i32 < b.as.as_i32) ? -1 : (a.as.as_i32 > b.as.as_i32) ? 1 : 0; break; - case VAL_I64: cmp = (a.as.as_i64 < b.as.as_i64) ? -1 : (a.as.as_i64 > b.as.as_i64) ? 1 : 0; break; - case VAL_U8: cmp = (a.as.as_u8 < b.as.as_u8) ? -1 : (a.as.as_u8 > b.as.as_u8) ? 1 : 0; break; - case VAL_U16: cmp = (a.as.as_u16 < b.as.as_u16) ? -1 : (a.as.as_u16 > b.as.as_u16) ? 1 : 0; break; - case VAL_U32: cmp = (a.as.as_u32 < b.as.as_u32) ? -1 : (a.as.as_u32 > b.as.as_u32) ? 1 : 0; break; - case VAL_U64: cmp = (a.as.as_u64 < b.as.as_u64) ? -1 : (a.as.as_u64 > b.as.as_u64) ? 1 : 0; break; - case VAL_F32: cmp = (a.as.as_f32 < b.as.as_f32) ? -1 : (a.as.as_f32 > b.as.as_f32) ? 1 : 0; break; - case VAL_F64: cmp = (a.as.as_f64 < b.as.as_f64) ? -1 : (a.as.as_f64 > b.as.as_f64) ? 1 : 0; break; - case VAL_BOOL: cmp = (int)a.as.as_bool - (int)b.as.as_bool; break; - case VAL_STRING: cmp = strcmp(a.as.as_string->data, b.as.as_string->data); break; - default: cmp = 0; break; // Objects, arrays, etc. - no default ordering - } - } - } - - if (cmp <= 0) break; - - arr->elements[j + 1] = arr->elements[j]; - j--; - } - arr->elements[j + 1] = key; + Value comparator = (num_args == 1) ? args[0] : val_null(); + if (!array_merge_sort(arr, comparator, num_args == 1, ctx)) { + return val_null(); // Comparator threw, or scratch allocation failed } return val_null(); } diff --git a/tests/parity/methods/array_sort.expected b/tests/parity/methods/array_sort.expected new file mode 100644 index 00000000..5e294e88 --- /dev/null +++ b/tests/parity/methods/array_sort.expected @@ -0,0 +1,22 @@ +=== default ordering === +[1, 1, 2, 3, 3, 4, 5, 5, 5, 6, 9] +[apple, apple, banana, cherry] +=== custom comparator === +[5, 4, 3, 1, 1] +=== edge cases === +[] +[7] +[1, 2] +[5, 5, 5, 5] +=== stability === +bdfhjlnacegikmo +=== presorted stays sorted === +first=0 last=1999 len=2000 +=== reversed input === +first=1 last=2000 len=2000 +=== shuffled input === +ordered=true len=2000 +=== throwing comparator === +caught: comparator failed +len=10 +[0, 1, 2, 3, 4, 5, 6, 7, 8, 9] diff --git a/tests/parity/methods/array_sort.hml b/tests/parity/methods/array_sort.hml new file mode 100644 index 00000000..204f969a --- /dev/null +++ b/tests/parity/methods/array_sort.hml @@ -0,0 +1,94 @@ +// Regression test for H-2: array.sort() must be O(n log n) and stable on +// every input. The compiled backend previously used a non-randomized Lomuto +// quicksort that degraded to O(n^2) time and O(n) recursion depth on sorted +// input, segfaulting on large presorted arrays; the interpreter used an +// insertion sort that degraded to O(n^2) on shuffled input. Both now use the +// same stable bottom-up merge sort. + +print("=== default ordering ==="); +let nums = [3, 1, 4, 1, 5, 9, 2, 6, 5, 3, 5]; +nums.sort(); +print(nums); + +let words = ["banana", "apple", "cherry", "apple"]; +words.sort(); +print(words); + +print("=== custom comparator ==="); +let desc = [3, 1, 4, 1, 5]; +desc.sort(fn(a, b) { return b - a; }); +print(desc); + +print("=== edge cases ==="); +let empty: array = []; +empty.sort(); +print(empty); + +let one = [7]; +one.sort(); +print(one); + +let two = [2, 1]; +two.sort(); +print(two); + +let equal = [5, 5, 5, 5]; +equal.sort(); +print(equal); + +print("=== stability ==="); +// Equal-comparing elements must keep their original relative order. The array +// is longer than one merge run so the result depends on the merge being stable. +let recs = [ + { k: 1, tag: "a" }, { k: 0, tag: "b" }, { k: 1, tag: "c" }, + { k: 0, tag: "d" }, { k: 1, tag: "e" }, { k: 0, tag: "f" }, + { k: 1, tag: "g" }, { k: 0, tag: "h" }, { k: 1, tag: "i" }, + { k: 0, tag: "j" }, { k: 1, tag: "k" }, { k: 0, tag: "l" }, + { k: 1, tag: "m" }, { k: 0, tag: "n" }, { k: 1, tag: "o" } +]; +recs.sort(fn(a, b) { return a.k - b.k; }); +let order = ""; +for (r in recs) { order = order + r.tag; } +print(order); + +print("=== presorted stays sorted ==="); +// The quicksort worst case: already-ascending input. +let asc = []; +for (let i = 0; i < 2000; i++) { asc.push(i); } +asc.sort(); +print("first=" + asc[0] + " last=" + asc[1999] + " len=" + asc.length); + +print("=== reversed input ==="); +// The other quicksort worst case: strictly descending input. +let desc2 = []; +for (let i = 2000; i > 0; i--) { desc2.push(i); } +desc2.sort(); +print("first=" + desc2[0] + " last=" + desc2[1999] + " len=" + desc2.length); + +print("=== shuffled input ==="); +// The insertion-sort worst case: pseudo-random input. +let big = []; +let seed: i64 = 12345; +for (let i = 0; i < 2000; i++) { + seed = (seed * 1103515245 + 12345) % 2147483647; + big.push(i32(seed % 10000)); +} +big.sort(); +let ordered = true; +for (let i = 1; i < big.length; i++) { + if (big[i - 1] > big[i]) { ordered = false; } +} +print("ordered=" + ordered + " len=" + big.length); + +print("=== throwing comparator ==="); +// A comparator that throws aborts the sort, but the array must remain a valid +// permutation of its elements - no slot duplicated, nothing lost. +let partial = [5, 3, 8, 1, 9, 2, 7, 4, 6, 0]; +try { + partial.sort(fn(a, b) { throw "comparator failed"; }); +} catch (e) { + print("caught: " + e); +} +print("len=" + partial.length); +partial.sort(); +print(partial);