From 203fee62f3c33d6c67e88b1c858ad3bcb6d9c76a Mon Sep 17 00:00:00 2001 From: Malo <57839069+MDLC01@users.noreply.github.com> Date: Fri, 10 Jan 2025 17:04:07 +0100 Subject: [PATCH] Use `glidesort` --- Cargo.lock | 7 ++ Cargo.toml | 1 + crates/typst-library/Cargo.toml | 1 + crates/typst-library/src/foundations/array.rs | 115 +----------------- tests/suite/foundations/array.typ | 2 +- 5 files changed, 13 insertions(+), 113 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2c0bfe138..caf9d063b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -863,6 +863,12 @@ dependencies = [ "weezl", ] +[[package]] +name = "glidesort" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2e102e6eb644d3e0b186fc161e4460417880a0a0b87d235f2e5b8fb30f2e9e0" + [[package]] name = "half" version = "2.4.1" @@ -2916,6 +2922,7 @@ dependencies = [ "ecow", "flate2", "fontdb", + "glidesort", "hayagriva", "icu_properties", "icu_provider", diff --git a/Cargo.toml b/Cargo.toml index b4f704f80..306f3ba75 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,6 +58,7 @@ env_proxy = "0.4" flate2 = "1" fontdb = { version = "0.21", default-features = false } fs_extra = "1.3" +glidesort = "0.1.2" hayagriva = "0.8" heck = "0.5" hypher = "0.1.4" diff --git a/crates/typst-library/Cargo.toml b/crates/typst-library/Cargo.toml index cc5e26712..c6331bced 100644 --- a/crates/typst-library/Cargo.toml +++ b/crates/typst-library/Cargo.toml @@ -29,6 +29,7 @@ csv = { workspace = true } ecow = { workspace = true } flate2 = { workspace = true } fontdb = { workspace = true } +glidesort = { workspace = true } hayagriva = { workspace = true } icu_properties = { workspace = true } icu_provider = { workspace = true } diff --git a/crates/typst-library/src/foundations/array.rs b/crates/typst-library/src/foundations/array.rs index a420ae186..340c3adc7 100644 --- a/crates/typst-library/src/foundations/array.rs +++ b/crates/typst-library/src/foundations/array.rs @@ -874,7 +874,9 @@ impl Array { None => ops::compare(&x, &y).at(span), } }; - sort(vec.make_mut(), |a, b| { + // We use `glidesort` instead of the standard library sorting algorithm + // to prevent panics (see https://github.com/typst/typst/pull/5627). + glidesort::sort_by(vec.make_mut(), |a, b| { // Until we get `try` blocks :) compare(a.clone(), b.clone()).unwrap_or_else(|err| { if result.is_ok() { @@ -1160,114 +1162,3 @@ fn out_of_bounds_no_default(index: i64, len: usize) -> EcoString { and no default value was specified", ) } - -/// Sorts a slice according to a comparison function. This sort is stable. -/// -/// As opposed to [`<[T]>::sort_by`], this does not panic. -/// -/// This is implemented with an in-place stable insertion sort. -fn sort(array: &mut [T], mut cmp: impl FnMut(&T, &T) -> Ordering) { - for i in 0..array.len() { - for j in (0..i).rev() { - if cmp(&array[j], &array[j + 1]) == Ordering::Greater { - array.swap(j, j + 1); - } else { - break; - } - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn sort_empty() { - let mut array = []; - sort(&mut array, Ordering::cmp); - assert_eq!(array, []); - } - - #[test] - fn sort_singleton() { - let mut array = [0]; - sort(&mut array, |x, y| x.cmp(y)); - assert_eq!(array, [0]); - } - - #[test] - fn sort_pair() { - let mut array = [0, 1]; - sort(&mut array, |x, y| x.cmp(y)); - assert_eq!(array, [0, 1]); - - let mut array = [1, 0]; - sort(&mut array, |x, y| x.cmp(y)); - assert_eq!(array, [0, 1]); - } - - #[test] - fn sort_triplet() { - let arrays = [[0, 1, 2], [1, 0, 2], [0, 2, 1], [1, 2, 0], [2, 0, 1], [2, 1, 0]]; - - for mut array in arrays { - sort(&mut array, |x, y| x.cmp(y)); - assert_eq!(array, [0, 1, 2]); - } - } - - #[test] - fn sort_quadruplet() { - let arrays = [ - [0, 1, 2, 3], - [1, 0, 2, 3], - [0, 2, 1, 3], - [1, 2, 0, 3], - [2, 0, 1, 3], - [2, 1, 0, 3], - [0, 1, 3, 2], - [1, 0, 3, 2], - [0, 2, 3, 1], - [1, 2, 3, 0], - [2, 0, 3, 1], - [2, 1, 3, 0], - [0, 3, 1, 2], - [1, 3, 0, 2], - [0, 3, 2, 1], - [1, 3, 2, 0], - [2, 3, 0, 1], - [2, 3, 1, 0], - [3, 0, 1, 2], - [3, 1, 0, 2], - [3, 0, 2, 1], - [3, 1, 2, 0], - [3, 2, 0, 1], - [3, 2, 1, 0], - ]; - - for mut array in arrays { - sort(&mut array, |x, y| x.cmp(y)); - assert_eq!(array, [0, 1, 2, 3]); - } - } - - fn compare_first(x: &(T, U), y: &(T, U)) -> Ordering { - x.0.cmp(&y.0) - } - - #[test] - fn random() { - for m in 0..=u8::MAX { - // Generates a sort of "pseudorandom" array. - // Xoring with `k` prevents the array from being sorted by default. - let k = 0b01010101; - let mut array: [_; 100] = - std::array::from_fn(|i| ((i as u8 ^ k) & m, i as u8 ^ k ^ m)); - let mut copy = array; - sort(&mut array, compare_first); - copy.sort_by(compare_first); - assert_eq!(array, copy); - } - } -} diff --git a/tests/suite/foundations/array.typ b/tests/suite/foundations/array.typ index 330ae774a..2fbd185d6 100644 --- a/tests/suite/foundations/array.typ +++ b/tests/suite/foundations/array.typ @@ -447,7 +447,7 @@ #([Hi], [There]).sorted() --- array-sorted-uncomparable-lengths --- -// Error: 2-26 cannot compare 2pt with 3em +// Error: 2-26 cannot compare 3em with 2pt #(1pt, 2pt, 3em).sorted() --- array-sorted-key-function-positional-2 ---