Small refactorings

docs/src/reference/groups.yml

@@ -64,7 +64,7 @@
 
 - name: lr
   display: Left/Right
-  functions: ["lr", "abs", "norm", "floor", "ceil"]
+  functions: ["lr", "abs", "norm", "floor", "ceil", "round"]
   description: |
     Delimiter matching.
 

library/src/compute/calc.rs

@@ -43,10 +43,10 @@ pub fn module() -> Module {
     scope.define("rem", rem_func());
     scope.define("mod", mod_func());
     scope.define("quo", quo_func());
-    scope.define("inf", Value::Float(f64::INFINITY));
+    scope.define("inf", f64::INFINITY);
-    scope.define("nan", Value::Float(f64::NAN));
+    scope.define("nan", f64::NAN);
-    scope.define("pi", Value::Float(std::f64::consts::PI));
+    scope.define("pi", std::f64::consts::PI);
-    scope.define("e", Value::Float(std::f64::consts::E));
+    scope.define("e", std::f64::consts::E);
     Module::new("calc").with_scope(scope)
 }
 

src/diag.rs

@@ -12,7 +12,7 @@ use comemo::Tracked;
 use crate::syntax::{ErrorPos, Span, Spanned};
 use crate::World;
 
-/// Early-return with a [`StrError`] or [`SourceError`].
+/// Early-return with a [`StrResult`] or [`SourceResult`].
 #[macro_export]
 #[doc(hidden)]
 macro_rules! __bail {
@@ -35,7 +35,7 @@ macro_rules! __bail {
 #[doc(inline)]
 pub use crate::__bail as bail;
 
-/// Construct a [`StrError`] or [`SourceError`].
+/// Construct an [`EcoString`] or [`SourceError`].
 #[macro_export]
 #[doc(hidden)]
 macro_rules! __error {

src/eval/func.rs

@@ -7,7 +7,8 @@ use ecow::eco_format;
 use once_cell::sync::Lazy;
 
 use super::{
-    cast, Args, CastInfo, Eval, Flow, IntoValue, Route, Scope, Scopes, Tracer, Value, Vm,
+    cast, Args, CastInfo, Eval, FlowEvent, IntoValue, Route, Scope, Scopes, Tracer,
+    Value, Vm,
 };
 use crate::diag::{bail, SourceResult, StrResult};
 use crate::model::{ElemFunc, Introspector, Locator, Vt};
@@ -404,8 +405,8 @@ impl Closure {
         // Handle control flow.
         let result = closure.body.eval(&mut vm);
         match vm.flow {
-            Some(Flow::Return(_, Some(explicit))) => return Ok(explicit),
+            Some(FlowEvent::Return(_, Some(explicit))) => return Ok(explicit),
-            Some(Flow::Return(_, None)) => {}
+            Some(FlowEvent::Return(_, None)) => {}
             Some(flow) => bail!(flow.forbidden()),
             None => {}
         }

src/eval/mod.rs

@@ -191,7 +191,7 @@ pub struct Vm<'a> {
     /// The current location.
     location: SourceId,
     /// A control flow event that is currently happening.
-    flow: Option<Flow>,
+    flow: Option<FlowEvent>,
     /// The stack of scopes.
     scopes: Scopes<'a>,
     /// The current call depth.
@@ -257,7 +257,7 @@ impl<'a> Vm<'a> {
 
 /// A control flow event that occurred during evaluation.
 #[derive(Debug, Clone, PartialEq)]
-pub enum Flow {
+pub enum FlowEvent {
     /// Stop iteration in a loop.
     Break(Span),
     /// Skip the remainder of the current iteration in a loop.
@@ -267,7 +267,7 @@ pub enum Flow {
     Return(Span, Option<Value>),
 }
 
-impl Flow {
+impl FlowEvent {
     /// Return an error stating that this control flow is forbidden.
     pub fn forbidden(&self) -> SourceError {
         match *self {
@@ -328,7 +328,7 @@ impl<'a> Route<'a> {
     }
 }
 
-/// Traces which values existed for the expression at a span.
+/// Traces which values existed for an expression at a span.
 #[derive(Default, Clone)]
 pub struct Tracer {
     span: Option<Span>,
@@ -1286,17 +1286,18 @@ impl Eval for ast::Closure {
 }
 
 impl ast::Pattern {
-    fn destruct_array<T>(
+    fn destruct_array<F>(
         &self,
         vm: &mut Vm,
         value: Array,
-        f: T,
+        f: F,
         destruct: &ast::Destructuring,
     ) -> SourceResult<Value>
     where
-        T: Fn(&mut Vm, ast::Expr, Value) -> SourceResult<Value>,
+        F: Fn(&mut Vm, ast::Expr, Value) -> SourceResult<Value>,
     {
         let mut i = 0;
+        let len = value.as_slice().len();
         for p in destruct.bindings() {
             match p {
                 ast::DestructuringKind::Normal(expr) => {
@@ -1307,14 +1308,11 @@ impl ast::Pattern {
                     i += 1;
                 }
                 ast::DestructuringKind::Sink(spread) => {
-                    let sink_size =
+                    let sink_size = (1 + len).checked_sub(destruct.bindings().count());
-                        (1 + value.len()).checked_sub(destruct.bindings().count());
+                    let sink = sink_size.and_then(|s| value.as_slice().get(i..i + s));
-                    let sink = sink_size
-                        .and_then(|s| value.slice(i as i64, Some((i + s) as i64)).ok());
-
                     if let (Some(sink_size), Some(sink)) = (sink_size, sink) {
                         if let Some(expr) = spread.expr() {
-                            f(vm, expr, Value::Array(sink.clone()))?;
+                            f(vm, expr, Value::Array(sink.into()))?;
                         }
                         i += sink_size;
                     } else {
@@ -1325,7 +1323,7 @@ impl ast::Pattern {
                     bail!(named.span(), "cannot destructure named elements from an array")
                 }
                 ast::DestructuringKind::Placeholder(underscore) => {
-                    if i < value.len() {
+                    if i < len {
                         i += 1
                     } else {
                         bail!(underscore.span(), "not enough elements to destructure")
@@ -1333,41 +1331,44 @@ impl ast::Pattern {
                 }
             }
         }
-        if i < value.len() {
+        if i < len {
             bail!(self.span(), "too many elements to destructure");
         }
 
         Ok(Value::None)
     }
 
-    fn destruct_dict<T>(
+    fn destruct_dict<F>(
         &self,
         vm: &mut Vm,
-        value: Dict,
+        dict: Dict,
-        f: T,
+        f: F,
         destruct: &ast::Destructuring,
     ) -> SourceResult<Value>
     where
-        T: Fn(&mut Vm, ast::Expr, Value) -> SourceResult<Value>,
+        F: Fn(&mut Vm, ast::Expr, Value) -> SourceResult<Value>,
     {
         let mut sink = None;
         let mut used = HashSet::new();
         for p in destruct.bindings() {
             match p {
                 ast::DestructuringKind::Normal(ast::Expr::Ident(ident)) => {
-                    let Ok(v) = value.at(&ident, None) else {
+                    let v = dict
-                        bail!(ident.span(), "destructuring key not found in dictionary");
+                        .at(&ident, None)
-                    };
+                        .map_err(|_| "destructuring key not found in dictionary")
+                        .at(ident.span())?;
                     f(vm, ast::Expr::Ident(ident.clone()), v.clone())?;
                     used.insert(ident.take());
                 }
                 ast::DestructuringKind::Sink(spread) => sink = spread.expr(),
                 ast::DestructuringKind::Named(named) => {
-                    let Ok(v) = value.at(named.name().as_str(), None) else {
+                    let name = named.name();
-                        bail!(named.name().span(), "destructuring key not found in dictionary");
+                    let v = dict
-                    };
+                        .at(&name, None)
+                        .map_err(|_| "destructuring key not found in dictionary")
+                        .at(name.span())?;
                     f(vm, named.expr(), v.clone())?;
-                    used.insert(named.name().take());
+                    used.insert(name.take());
                 }
                 ast::DestructuringKind::Placeholder(_) => {}
                 ast::DestructuringKind::Normal(expr) => {
@@ -1378,7 +1379,7 @@ impl ast::Pattern {
 
         if let Some(expr) = sink {
             let mut sink = Dict::new();
-            for (key, value) in value {
+            for (key, value) in dict {
                 if !used.contains(key.as_str()) {
                     sink.insert(key, value);
                 }
@@ -1549,12 +1550,12 @@ impl Eval for ast::WhileLoop {
             output = ops::join(output, value).at(body.span())?;
 
             match vm.flow {
-                Some(Flow::Break(_)) => {
+                Some(FlowEvent::Break(_)) => {
                     vm.flow = None;
                     break;
                 }
-                Some(Flow::Continue(_)) => vm.flow = None,
+                Some(FlowEvent::Continue(_)) => vm.flow = None,
-                Some(Flow::Return(..)) => break,
+                Some(FlowEvent::Return(..)) => break,
                 None => {}
             }
 
@@ -1612,12 +1613,12 @@ impl Eval for ast::ForLoop {
                     output = ops::join(output, value).at(body.span())?;
 
                     match vm.flow {
-                        Some(Flow::Break(_)) => {
+                        Some(FlowEvent::Break(_)) => {
                             vm.flow = None;
                             break;
                         }
-                        Some(Flow::Continue(_)) => vm.flow = None,
+                        Some(FlowEvent::Continue(_)) => vm.flow = None,
-                        Some(Flow::Return(..)) => break,
+                        Some(FlowEvent::Return(..)) => break,
                         None => {}
                     }
                 }
@@ -1781,7 +1782,7 @@ impl Eval for ast::LoopBreak {
     #[tracing::instrument(name = "LoopBreak::eval", skip_all)]
     fn eval(&self, vm: &mut Vm) -> SourceResult<Self::Output> {
         if vm.flow.is_none() {
-            vm.flow = Some(Flow::Break(self.span()));
+            vm.flow = Some(FlowEvent::Break(self.span()));
         }
         Ok(Value::None)
     }
@@ -1793,7 +1794,7 @@ impl Eval for ast::LoopContinue {
     #[tracing::instrument(name = "LoopContinue::eval", skip_all)]
     fn eval(&self, vm: &mut Vm) -> SourceResult<Self::Output> {
         if vm.flow.is_none() {
-            vm.flow = Some(Flow::Continue(self.span()));
+            vm.flow = Some(FlowEvent::Continue(self.span()));
         }
         Ok(Value::None)
     }
@@ -1806,7 +1807,7 @@ impl Eval for ast::FuncReturn {
     fn eval(&self, vm: &mut Vm) -> SourceResult<Self::Output> {
         let value = self.body().map(|body| body.eval(vm)).transpose()?;
         if vm.flow.is_none() {
-            vm.flow = Some(Flow::Return(self.span(), value));
+            vm.flow = Some(FlowEvent::Return(self.span(), value));
         }
         Ok(Value::None)
     }

src/geom/rounded.rs

@@ -1,7 +1,5 @@
 use super::*;
 
-use std::mem;
-
 /// Produce shapes that together make up a rounded rectangle.
 pub fn rounded_rect(
     size: Size,
@@ -69,7 +67,7 @@ fn stroke_segments(
         );
 
         if !continuous {
-            res.push((mem::take(&mut path), stroke.get_ref(side).clone()));
+            res.push((std::mem::take(&mut path), stroke.get_ref(side).clone()));
         }
     }
 

src/geom/scalar.rs

@@ -34,6 +34,21 @@ impl Debug for Scalar {
     }
 }
 
+impl Eq for Scalar {}
+
+impl PartialEq for Scalar {
+    fn eq(&self, other: &Self) -> bool {
+        assert!(!self.0.is_nan() && !other.0.is_nan(), "float is NaN");
+        self.0 == other.0
+    }
+}
+
+impl PartialEq<f64> for Scalar {
+    fn eq(&self, other: &f64) -> bool {
+        self == &Self(*other)
+    }
+}
+
 impl Ord for Scalar {
     fn cmp(&self, other: &Self) -> Ordering {
         self.partial_cmp(other).expect("float is NaN")
@@ -62,21 +77,6 @@ impl PartialOrd for Scalar {
     }
 }
 
-impl Eq for Scalar {}
-
-impl PartialEq for Scalar {
-    fn eq(&self, other: &Self) -> bool {
-        assert!(!self.0.is_nan() && !other.0.is_nan(), "float is NaN");
-        self.0 == other.0
-    }
-}
-
-impl PartialEq<f64> for Scalar {
-    fn eq(&self, other: &f64) -> bool {
-        self == &Self(*other)
-    }
-}
-
 impl Hash for Scalar {
     fn hash<H: Hasher>(&self, state: &mut H) {
         debug_assert!(!self.0.is_nan(), "float is NaN");