From a2fcc1bf288c5162de7b2158166de62cb0610083 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Thu, 11 Feb 2021 21:22:06 +0100 Subject: [PATCH] =?UTF-8?q?Small=20fixes=20=E2=99=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- bench/src/bench.rs | 2 +- src/eval/capture.rs | 4 +- src/eval/mod.rs | 112 +++++++++++++++++++++----------------------- src/eval/value.rs | 23 ++++----- src/export/pdf.rs | 6 +-- src/main.rs | 2 +- tests/typeset.rs | 18 +++---- 7 files changed, 82 insertions(+), 85 deletions(-) diff --git a/bench/src/bench.rs b/bench/src/bench.rs index 88e2388d9..0999b2467 100644 --- a/bench/src/bench.rs +++ b/bench/src/bench.rs @@ -44,7 +44,7 @@ fn benchmarks(c: &mut Criterion) { bench!("exec-coma": exec(&mut env, &syntax_tree, &expr_map, state.clone())); bench!("layout-coma": layout(&mut env, &layout_tree)); bench!("typeset-coma": typeset(&mut env, COMA, &scope, state.clone())); - bench!("export-pdf-coma": pdf::export(&frames, &env)); + bench!("export-pdf-coma": pdf::export(&env, &frames)); } criterion_group!(benches, benchmarks); diff --git a/src/eval/capture.rs b/src/eval/capture.rs index bbabc5031..163aa24ef 100644 --- a/src/eval/capture.rs +++ b/src/eval/capture.rs @@ -16,12 +16,12 @@ impl<'a> CapturesVisitor<'a> { pub fn new(external: &'a Scopes) -> Self { Self { external, - internal: Scopes::default(), + internal: Scopes::new(), captures: Scope::new(), } } - /// Return the scope of capture variables. + /// Return the scope of captured variables. pub fn finish(self) -> Scope { self.captures } diff --git a/src/eval/mod.rs b/src/eval/mod.rs index 57a37f382..47f997eb4 100644 --- a/src/eval/mod.rs +++ b/src/eval/mod.rs @@ -30,7 +30,7 @@ pub fn eval(env: &mut Env, tree: &Tree, scope: &Scope) -> Pass { Pass::new(map, ctx.diags) } -/// A map from expression to values to evaluated to. +/// A map from expressions to the values they evaluated to. /// /// The raw pointers point into the expressions contained in some [tree](Tree). /// Since the lifetime is erased, the tree could go out of scope while the hash @@ -89,7 +89,7 @@ impl Eval for Tree { } } - let mut visitor = ExprVisitor { map: HashMap::new(), ctx }; + let mut visitor = ExprVisitor { map: ExprMap::new(), ctx }; visitor.visit_tree(self); visitor.map } @@ -260,9 +260,8 @@ impl ExprBinary { where F: FnOnce(Value, Value) -> Value, { - let lhs = self.lhs.eval(ctx); - // Short-circuit boolean operations. + let lhs = self.lhs.eval(ctx); match (self.op, &lhs) { (BinOp::And, Value::Bool(false)) => return lhs, (BinOp::Or, Value::Bool(true)) => return lhs, @@ -270,16 +269,15 @@ impl ExprBinary { } let rhs = self.rhs.eval(ctx); - if lhs == Value::Error || rhs == Value::Error { return Value::Error; } - let (l, r) = (lhs.type_name(), rhs.type_name()); - + // Save type names before we consume the values in case of error. + let types = (lhs.type_name(), rhs.type_name()); let out = op(lhs, rhs); if out == Value::Error { - self.error(ctx, l, r); + self.error(ctx, types); } out @@ -290,54 +288,47 @@ impl ExprBinary { where F: FnOnce(Value, Value) -> Value, { - let rhs = self.rhs.eval(ctx); - - let lhs_span = self.lhs.span(); let slot = if let Expr::Ident(id) = self.lhs.as_ref() { match ctx.scopes.get(id) { - Some(slot) => slot, + Some(slot) => Rc::clone(slot), None => { - ctx.diag(error!(lhs_span, "unknown variable")); + ctx.diag(error!(self.lhs.span(), "unknown variable")); return Value::Error; } } } else { - ctx.diag(error!(lhs_span, "cannot assign to this expression")); + ctx.diag(error!(self.lhs.span(), "cannot assign to this expression")); return Value::Error; }; - let (constant, err, value) = if let Ok(mut inner) = slot.try_borrow_mut() { - let lhs = std::mem::take(&mut *inner); - let types = (lhs.type_name(), rhs.type_name()); - - *inner = op(lhs, rhs); - if *inner == Value::Error { - (false, Some(types), Value::Error) - } else { - (false, None, Value::None) + let rhs = self.rhs.eval(ctx); + let mut mutable = match slot.try_borrow_mut() { + Ok(mutable) => mutable, + Err(_) => { + ctx.diag(error!(self.lhs.span(), "cannot assign to a constant")); + return Value::Error; } - } else { - (true, None, Value::Error) }; - if constant { - ctx.diag(error!(lhs_span, "cannot assign to a constant")); + let lhs = std::mem::take(&mut *mutable); + let types = (lhs.type_name(), rhs.type_name()); + *mutable = op(lhs, rhs); + + if *mutable == Value::Error { + self.error(ctx, types); + return Value::Error; } - if let Some((l, r)) = err { - self.error(ctx, l, r); - } - - value + Value::None } - fn error(&self, ctx: &mut EvalContext, l: &str, r: &str) { + fn error(&self, ctx: &mut EvalContext, (a, b): (&str, &str)) { ctx.diag(error!(self.span, "{}", match self.op { - BinOp::Add => format!("cannot add {} and {}", l, r), - BinOp::Sub => format!("cannot subtract {1} from {0}", l, r), - BinOp::Mul => format!("cannot multiply {} with {}", l, r), - BinOp::Div => format!("cannot divide {} by {}", l, r), - _ => format!("cannot apply '{}' to {} and {}", self.op.as_str(), l, r), + BinOp::Add => format!("cannot add {} and {}", a, b), + BinOp::Sub => format!("cannot subtract {1} from {0}", a, b), + BinOp::Mul => format!("cannot multiply {} with {}", a, b), + BinOp::Div => format!("cannot divide {} by {}", a, b), + _ => format!("cannot apply '{}' to {} and {}", self.op.as_str(), a, b), })); } } @@ -350,6 +341,7 @@ impl Eval for ExprCall { if let Value::Func(func) = callee { let func = func.clone(); + let mut args = self.args.eval(ctx); let returned = func(ctx, &mut args); args.finish(ctx); @@ -371,25 +363,28 @@ impl Eval for ExprArgs { type Output = ValueArgs; fn eval(&self, ctx: &mut EvalContext) -> Self::Output { - let items = self - .items - .iter() - .map(|arg| match arg { - ExprArg::Pos(expr) => ValueArg { - name: None, - value: Spanned::new(expr.eval(ctx), expr.span()), - }, - ExprArg::Named(Named { name, expr }) => ValueArg { - name: Some(Spanned::new(name.string.clone(), name.span)), - value: Spanned::new(expr.eval(ctx), expr.span()), - }, - }) - .collect(); - + let items = self.items.iter().map(|arg| arg.eval(ctx)).collect(); ValueArgs { span: self.span, items } } } +impl Eval for ExprArg { + type Output = ValueArg; + + fn eval(&self, ctx: &mut EvalContext) -> Self::Output { + match self { + Self::Pos(expr) => ValueArg { + name: None, + value: Spanned::new(expr.eval(ctx), expr.span()), + }, + Self::Named(Named { name, expr }) => ValueArg { + name: Some(Spanned::new(name.string.clone(), name.span)), + value: Spanned::new(expr.eval(ctx), expr.span()), + }, + } + } +} + impl Eval for ExprLet { type Output = Value; @@ -408,6 +403,7 @@ impl Eval for ExprIf { fn eval(&self, ctx: &mut EvalContext) -> Self::Output { let condition = self.condition.eval(ctx); + if let Value::Bool(boolean) = condition { return if boolean { self.if_body.eval(ctx) @@ -432,7 +428,7 @@ impl Eval for ExprFor { type Output = Value; fn eval(&self, ctx: &mut EvalContext) -> Self::Output { - macro_rules! iterate { + macro_rules! iter { (for ($($binding:ident => $value:ident),*) in $iter:expr) => {{ let mut output = vec![]; @@ -454,16 +450,16 @@ impl Eval for ExprFor { let iter = self.iter.eval(ctx); let value = match (self.pattern.clone(), iter) { (ForPattern::Value(v), Value::Str(string)) => { - iterate!(for (v => value) in string.chars().map(|c| Value::Str(c.into()))) + iter!(for (v => value) in string.chars().map(|c| Value::Str(c.into()))) } (ForPattern::Value(v), Value::Array(array)) => { - iterate!(for (v => value) in array.into_iter()) + iter!(for (v => value) in array.into_iter()) } (ForPattern::Value(v), Value::Dict(dict)) => { - iterate!(for (v => value) in dict.into_iter().map(|p| p.1)) + iter!(for (v => value) in dict.into_iter().map(|p| p.1)) } (ForPattern::KeyValue(k, v), Value::Dict(dict)) => { - iterate!(for (k => key, v => value) in dict.into_iter()) + iter!(for (k => key, v => value) in dict.into_iter()) } (ForPattern::KeyValue(_, _), Value::Str(_)) diff --git a/src/eval/value.rs b/src/eval/value.rs index ab54bf538..ce89d4538 100644 --- a/src/eval/value.rs +++ b/src/eval/value.rs @@ -124,16 +124,16 @@ pub type ValueTemplate = Vec; /// One chunk of a template. /// -/// Evaluating a template expression creates only a single chunk. Adding two -/// such templates yields a two-chunk template. +/// Evaluating a template expression creates only a single node. Adding multiple +/// templates can yield multi-node templates. #[derive(Debug, Clone, PartialEq)] pub enum TemplateNode { /// A template that consists of a syntax tree plus already evaluated /// expression. Tree { - /// The tree of this template part. + /// The syntax tree of the corresponding template expression. tree: Rc, - /// The evaluated expressions. + /// The evaluated expressions for the `tree`. map: ExprMap, }, /// A template that can implement custom behaviour. @@ -242,7 +242,7 @@ impl ValueArgs { where T: Cast>, { - (0 .. self.items.len()).find_map(move |i| self.try_take(ctx, i)) + (0 .. self.items.len()).find_map(move |i| self.try_take(&mut ctx.diags, i)) } /// Find and remove the first convertible positional argument, producing an @@ -259,17 +259,18 @@ impl ValueArgs { } /// Filter out and remove all convertible positional arguments. - pub fn filter<'a, 'b: 'a, T>( + pub fn filter<'a, T>( &'a mut self, - ctx: &'a mut EvalContext<'b>, - ) -> impl Iterator + Captures<'a> + Captures<'b> + ctx: &'a mut EvalContext, + ) -> impl Iterator + 'a where T: Cast>, { + let diags = &mut ctx.diags; let mut i = 0; std::iter::from_fn(move || { while i < self.items.len() { - if let Some(val) = self.try_take(ctx, i) { + if let Some(val) = self.try_take(diags, i) { return Some(val); } i += 1; @@ -328,7 +329,7 @@ impl ValueArgs { /// Try to take and cast a positional argument in the i'th slot into `T`, /// putting it back if the conversion fails. - fn try_take(&mut self, ctx: &mut EvalContext, i: usize) -> Option + fn try_take(&mut self, diags: &mut DiagSet, i: usize) -> Option where T: Cast>, { @@ -346,7 +347,7 @@ impl ValueArgs { } CastResult::Warn(t, m) => { self.items.remove(i); - ctx.diag(warning!(span, "{}", m)); + diags.insert(warning!(span, "{}", m)); Some(t) } CastResult::Err(value) => { diff --git a/src/export/pdf.rs b/src/export/pdf.rs index b2a6bbfd0..9f3552786 100644 --- a/src/export/pdf.rs +++ b/src/export/pdf.rs @@ -25,8 +25,8 @@ use crate::layout::{Element, Fill, Frame, Shape}; /// images can be included in the _PDF_. /// /// Returns the raw bytes making up the _PDF_ document. -pub fn export(frames: &[Frame], env: &Env) -> Vec { - PdfExporter::new(frames, env).write() +pub fn export(env: &Env, frames: &[Frame]) -> Vec { + PdfExporter::new(env, frames).write() } struct PdfExporter<'a> { @@ -39,7 +39,7 @@ struct PdfExporter<'a> { } impl<'a> PdfExporter<'a> { - fn new(frames: &'a [Frame], env: &'a Env) -> Self { + fn new(env: &'a Env, frames: &'a [Frame]) -> Self { let mut writer = PdfWriter::new(1, 7); writer.set_indent(2); diff --git a/src/main.rs b/src/main.rs index c221f3b61..ae655253c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -65,7 +65,7 @@ fn main() -> anyhow::Result<()> { } } - let pdf_data = pdf::export(&frames, &env); + let pdf_data = pdf::export(&env, &frames); fs::write(&dest_path, pdf_data).context("Failed to write PDF file.")?; Ok(()) diff --git a/tests/typeset.rs b/tests/typeset.rs index 8932c483b..7f8646bee 100644 --- a/tests/typeset.rs +++ b/tests/typeset.rs @@ -168,11 +168,11 @@ fn test( } if !frames.is_empty() { - let pdf_data = pdf::export(&frames, &env); + let pdf_data = pdf::export(&env, &frames); fs::create_dir_all(&pdf_path.parent().unwrap()).unwrap(); fs::write(pdf_path, pdf_data).unwrap(); - let canvas = draw(&frames, &env, 2.0); + let canvas = draw(&env, &frames, 2.0); fs::create_dir_all(&png_path.parent().unwrap()).unwrap(); canvas.pixmap.save_png(png_path).unwrap(); @@ -339,7 +339,7 @@ fn print_diag(diag: &Diag, map: &LineMap, lines: u32) { println!("{}: {}-{}: {}", diag.level, start, end, diag.message); } -fn draw(frames: &[Frame], env: &Env, pixel_per_pt: f32) -> Canvas { +fn draw(env: &Env, frames: &[Frame], pixel_per_pt: f32) -> Canvas { let pad = Length::pt(5.0); let height = pad + frames.iter().map(|l| l.size.height + pad).sum::(); @@ -380,13 +380,13 @@ fn draw(frames: &[Frame], env: &Env, pixel_per_pt: f32) -> Canvas { let pos = origin + pos; match element { Element::Text(shaped) => { - draw_text(&mut canvas, pos, env, shaped); + draw_text(env, &mut canvas, pos, shaped); } Element::Image(image) => { - draw_image(&mut canvas, pos, env, image); + draw_image(env, &mut canvas, pos, image); } Element::Geometry(geom) => { - draw_geometry(&mut canvas, pos, env, geom); + draw_geometry(env, &mut canvas, pos, geom); } } } @@ -397,7 +397,7 @@ fn draw(frames: &[Frame], env: &Env, pixel_per_pt: f32) -> Canvas { canvas } -fn draw_text(canvas: &mut Canvas, pos: Point, env: &Env, shaped: &Shaped) { +fn draw_text(env: &Env, canvas: &mut Canvas, pos: Point, shaped: &Shaped) { let face = env.fonts.face(shaped.face).get(); for (&glyph, &offset) in shaped.glyphs.iter().zip(&shaped.offsets) { @@ -423,7 +423,7 @@ fn draw_text(canvas: &mut Canvas, pos: Point, env: &Env, shaped: &Shaped) { } } -fn draw_geometry(canvas: &mut Canvas, pos: Point, _: &Env, element: &Geometry) { +fn draw_geometry(_: &Env, canvas: &mut Canvas, pos: Point, element: &Geometry) { let x = pos.x.to_pt() as f32; let y = pos.y.to_pt() as f32; @@ -443,7 +443,7 @@ fn draw_geometry(canvas: &mut Canvas, pos: Point, _: &Env, element: &Geometry) { }; } -fn draw_image(canvas: &mut Canvas, pos: Point, env: &Env, element: &Image) { +fn draw_image(env: &Env, canvas: &mut Canvas, pos: Point, element: &Image) { let img = &env.resources.loaded::(element.res); let mut pixmap = Pixmap::new(img.buf.width(), img.buf.height()).unwrap();