about summary refs log tree commit diff
path: root/tvix/eval/src/builtins
diff options
context:
space:
mode:
authorAspen Smith <root@gws.fyi>2024-02-08T21·52-0500
committeraspen <root@gws.fyi>2024-02-08T22·50+0000
commit8446cd1c8b5747738264dea9acd2be76e2db054e (patch)
tree69d5298c4dad04da84ce02babc8a61f5656e390c /tvix/eval/src/builtins
parentddb7bc8d18bd11deb87a70ec300a281aaf34c2c7 (diff)
fix(tvix/eval): Inline List.sort_by, and propagate errors r/7488
In order to correctly propagate errors in the comparator passed to
builtins.sort, we need to do all the sorting in a context where we can
short-circuit return `Value`s (because catchables are Values on the `Ok`
side of the Result , not `Err`s). Unfortunately this means we have
to *inline* the List `sort_by` implementation into the builtin_sort
function - fortunately this is the only place that was called so this is
relatively low cost. This does that, and adds the requisite `try_value!`
invocation to allow us to propagate comparator errors here.

As before, this doesn't include tests, primarily since those are coming
in the next commit.

Change-Id: I8453c3aa2cd82299eae89828e2a2bb118da4cd48
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10754
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Diffstat (limited to 'tvix/eval/src/builtins')
-rw-r--r--tvix/eval/src/builtins/mod.rs48
1 files changed, 45 insertions, 3 deletions
diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index 716da4d04156..d9f8567886e3 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -87,7 +87,7 @@ mod pure_builtins {
     use itertools::Itertools;
     use os_str_bytes::OsStringBytes;
 
-    use crate::{value::PointerEquality, NixContext, NixContextElement};
+    use crate::{value::PointerEquality, AddContext, NixContext, NixContextElement};
 
     use super::*;
 
@@ -1223,8 +1223,50 @@ mod pure_builtins {
     #[builtin("sort")]
     async fn builtin_sort(co: GenCo, comparator: Value, list: Value) -> Result<Value, ErrorKind> {
         let list = list.to_list()?;
-        let sorted = list.sort_by(&co, comparator).await?;
-        Ok(Value::List(sorted))
+        let mut len = list.len();
+        let mut data = list.into_inner();
+
+        // Asynchronous sorting algorithm in which the comparator can make use of
+        // VM requests (required as `builtins.sort` uses comparators written in
+        // Nix).
+        //
+        // This is a simple, optimised bubble sort implementation. The choice of
+        // algorithm is constrained by the comparator in Nix not being able to
+        // yield equality, and us being unable to use the standard library
+        // implementation of sorting (which is a lot longer, but a lot more
+        // efficient) here.
+        // TODO(amjoseph): Investigate potential impl in Nix code, or Tvix bytecode.
+        loop {
+            let mut new_len = 0;
+            for i in 1..len {
+                if try_value!(
+                    generators::request_force(
+                        &co,
+                        generators::request_call_with(
+                            &co,
+                            comparator.clone(),
+                            [data[i].clone(), data[i - 1].clone()],
+                        )
+                        .await,
+                    )
+                    .await
+                )
+                .as_bool()
+                .context("evaluating comparator in `builtins.sort`")?
+                {
+                    data.swap(i, i - 1);
+                    new_len = i;
+                }
+            }
+
+            if new_len == 0 {
+                break;
+            }
+
+            len = new_len;
+        }
+
+        Ok(Value::List(data.into()))
     }
 
     #[builtin("splitVersion")]