From b13ed627fff73a599b34d760cd99aa2f08d58ea8 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Sat, 30 Nov 2019 14:10:35 +0100 Subject: [PATCH] =?UTF-8?q?Better=20error=20reporting=20=F0=9F=9A=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/func/helpers.rs | 28 +++++++++++------ src/layout/flex.rs | 18 ++--------- src/layout/mod.rs | 47 ++++++++--------------------- src/layout/{stacked.rs => stack.rs} | 2 +- src/layout/text.rs | 2 +- src/layout/tree.rs | 36 +++++++--------------- src/lib.rs | 4 +-- src/library/{axes.rs => align.rs} | 10 ++++-- src/library/mod.rs | 2 +- src/library/spacing.rs | 2 +- src/syntax/mod.rs | 1 + src/syntax/parsing.rs | 17 ++++++----- tests/layouting.rs | 41 ++++++++++++++----------- 13 files changed, 93 insertions(+), 117 deletions(-) rename src/layout/{stacked.rs => stack.rs} (98%) rename src/library/{axes.rs => align.rs} (92%) diff --git a/src/func/helpers.rs b/src/func/helpers.rs index 6e6896c9f..c82a90e1f 100644 --- a/src/func/helpers.rs +++ b/src/func/helpers.rs @@ -20,7 +20,7 @@ macro_rules! function { where Self: Sized { ArgParser::new(&header.args).done()?; if body.is_some() { - err!("expected no body"); + perr!("expected no body"); } Ok($ident) } @@ -63,7 +63,7 @@ macro_rules! function { macro_rules! parse { (forbidden: $body:expr) => { if $body.is_some() { - err!("unexpected body"); + perr!("unexpected body"); } }; @@ -79,7 +79,7 @@ macro_rules! parse { if let Some(body) = $body { $crate::syntax::parse(body, $ctx)? } else { - err!("expected body"); + perr!("expected body"); } ) } @@ -87,11 +87,20 @@ macro_rules! parse { /// Early-return with a formatted parsing error or yield /// an error expression without returning when prefixed with `@`. #[macro_export] -macro_rules! err { +macro_rules! perr { (@$($tts:tt)*) => ($crate::syntax::ParseError::new(format!($($tts)*))); - ($($tts:tt)*) => (return Err(err!(@$($tts)*));); + ($($tts:tt)*) => (return Err(perr!(@$($tts)*));); } +/// Early-return with a formatted layouting error or yield +/// an error expression without returning when prefixed with `@`. +#[macro_export] +macro_rules! lerr { + (@$($tts:tt)*) => ($crate::layout::LayoutError::new(format!($($tts)*))); + ($($tts:tt)*) => (return Err(lerr!(@$($tts)*));); +} + + /// Easy parsing of function arguments. pub struct ArgParser<'a> { args: &'a FuncArgs, @@ -112,7 +121,7 @@ impl<'a> ArgParser<'a> { /// this will return an error. pub fn get_pos(&mut self) -> ParseResult> where T: Argument<'a> { self.get_pos_opt::()? - .ok_or_else(|| err!(@"expected {}", T::ERROR_MESSAGE)) + .ok_or_else(|| perr!(@"expected {}", T::ERROR_MESSAGE)) } /// Get the next positional argument if there is any. @@ -136,7 +145,7 @@ impl<'a> ArgParser<'a> { pub fn get_key(&mut self, key: &str) -> ParseResult> where T: Argument<'a> { self.get_key_opt::(key)? - .ok_or_else(|| err!(@"expected {}", T::ERROR_MESSAGE)) + .ok_or_else(|| perr!(@"expected {}", T::ERROR_MESSAGE)) } /// Get a keyword argument with the given key and type if it is present. @@ -153,7 +162,7 @@ impl<'a> ArgParser<'a> { if self.positional_index == self.args.positional.len() { Ok(()) } else { - err!("unexpected argument"); + perr!("unexpected argument"); } } } @@ -176,9 +185,10 @@ macro_rules! arg { const ERROR_MESSAGE: &'static str = $err; fn from_expr(expr: &'a Spanned) -> ParseResult> { + #[allow(unreachable_patterns)] match &expr.val { $wanted => Ok(Spanned::new($converted, expr.span)), - #[allow(unreachable_patterns)] _ => err!("expected {}", $err), + _ => perr!("expected {}", $err), } } } diff --git a/src/layout/flex.rs b/src/layout/flex.rs index 053954a46..53f6dfdfb 100644 --- a/src/layout/flex.rs +++ b/src/layout/flex.rs @@ -126,20 +126,8 @@ impl FlexLayouter { } } - pub fn remaining(&self) -> LayoutResult<(LayoutSpaces, Option)> { - if self.run_is_empty() { - Ok((self.stack.remaining(), None)) - } else { - let mut future = self.clone(); - let remaining_run = future.finish_run()?; - - let stack_spaces = future.stack.remaining(); - let mut flex_spaces = stack_spaces.clone(); - flex_spaces[0].dimensions.x = remaining_run.x; - flex_spaces[0].dimensions.y += remaining_run.y; - - Ok((flex_spaces, Some(stack_spaces))) - } + pub fn remaining(&self) -> LayoutSpaces { + self.stack.remaining() } pub fn run_is_empty(&self) -> bool { @@ -242,7 +230,7 @@ impl FlexLayouter { while size.x > self.line.usable { if self.stack.space_is_last() { - Err(LayoutError::NotEnoughSpace("failed to add box to flex run"))?; + lerr!("box does not fit into line"); } self.stack.finish_space(true); diff --git a/src/layout/mod.rs b/src/layout/mod.rs index 2e6432956..cd4986d9f 100644 --- a/src/layout/mod.rs +++ b/src/layout/mod.rs @@ -15,14 +15,14 @@ use crate::syntax::{FuncCall, Node, SyntaxTree}; mod actions; mod tree; mod flex; -mod stacked; +mod stack; mod text; /// Different kinds of layouters (fully re-exported). pub mod layouters { pub use super::tree::layout_tree; pub use super::flex::{FlexLayouter, FlexContext}; - pub use super::stacked::{StackLayouter, StackContext}; + pub use super::stack::{StackLayouter, StackContext}; pub use super::text::{layout_text, TextContext}; } @@ -130,23 +130,17 @@ pub struct LayoutContext<'a, 'p> { /// The font loader to retrieve fonts from when typesetting text /// using [`layout_text`]. pub loader: &'a SharedFontLoader<'p>, - /// Whether this layouting process handles the top-level pages. pub top_level: bool, - /// The style to set text with. This includes sizes and font classes /// which determine which font from the loaders selection is used. pub text_style: &'a TextStyle, - /// The current size and margins of the top-level pages. pub page_style: PageStyle, - /// The spaces to layout in. pub spaces: LayoutSpaces, - /// The axes to flow on. pub axes: LayoutAxes, - /// Whether layouts should expand to the full dimensions of the space /// they lie on or whether should tightly fit the content. pub expand: bool, @@ -160,7 +154,6 @@ pub type LayoutSpaces = SmallVec<[LayoutSpace; 2]>; pub struct LayoutSpace { /// The maximum size of the box to layout in. pub dimensions: Size2D, - /// Padding that should be respected on each side. pub padding: SizeBox, } @@ -331,35 +324,21 @@ impl SpaceState { } /// The error type for layouting. -pub enum LayoutError { - /// An action is unallowed in the active context. - Unallowed(&'static str), - /// A specifier or operation is invalid for the given axis. - UnalignedAxis(&'static str), - /// There is not enough space to add an item. - NotEnoughSpace(&'static str), - /// There was no suitable font for the given character. - NoSuitableFont(char), - /// An error occured while gathering font data. - Font(FontError), -} +pub struct LayoutError(String); /// The result type for layouting. pub type LayoutResult = Result; +impl LayoutError { + /// Create a new layout error with a message. + pub fn new>(message: S) -> LayoutError { + LayoutError(message.into()) + } +} + error_type! { err: LayoutError, - show: f => match err { - LayoutError::Unallowed(desc) => write!(f, "unallowed: {}", desc), - LayoutError::UnalignedAxis(desc) => write!(f, "unaligned axis: {}", desc), - LayoutError::NotEnoughSpace(desc) => write!(f, "not enough space: {}", desc), - LayoutError::NoSuitableFont(c) => write!(f, "no suitable font for '{}'", c), - LayoutError::Font(err) => write!(f, "font error: {}", err), - }, - source: match err { - LayoutError::Font(err) => Some(err), - _ => None, - }, - from: (std::io::Error, LayoutError::Font(FontError::Io(err))), - from: (FontError, LayoutError::Font(err)), + show: f => f.write_str(&err.0), + from: (std::io::Error, LayoutError::new(err.to_string())), + from: (FontError, LayoutError::new(err.to_string())), } diff --git a/src/layout/stacked.rs b/src/layout/stack.rs similarity index 98% rename from src/layout/stacked.rs rename to src/layout/stack.rs index 2e128a5fe..f46c3da05 100644 --- a/src/layout/stacked.rs +++ b/src/layout/stack.rs @@ -95,7 +95,7 @@ impl StackLayouter { while !self.sub.usable.fits(new_dimensions) { if self.space_is_last() && self.space_is_empty() { - Err(LayoutError::NotEnoughSpace("failed to add box to stack"))?; + lerr!("box does not fit into stack"); } self.finish_space(true); diff --git a/src/layout/text.rs b/src/layout/text.rs index fc7cd3859..66ff75fd4 100644 --- a/src/layout/text.rs +++ b/src/layout/text.rs @@ -114,6 +114,6 @@ impl<'a, 'p> TextLayouter<'a, 'p> { self.classes.pop(); } - Err(LayoutError::NoSuitableFont(c)) + lerr!("no suitable font for character `{}`", c); } } diff --git a/src/layout/tree.rs b/src/layout/tree.rs index f5ae24351..56fb120c6 100644 --- a/src/layout/tree.rs +++ b/src/layout/tree.rs @@ -66,34 +66,20 @@ impl<'a, 'p> TreeLayouter<'a, 'p> { } fn layout_func(&mut self, func: &FuncCall) -> LayoutResult<()> { - let (first, second) = self.flex.remaining()?; + let spaces = self.flex.remaining(); let mut axes = self.ctx.axes.expanding(false); axes.secondary.alignment = Alignment::Origin; - let ctx = |spaces| { - LayoutContext { - loader: self.ctx.loader, - top_level: false, - text_style: &self.style, - page_style: self.ctx.page_style, - spaces, - axes, - expand: false, - } - }; - - let commands = match func.body.val.layout(ctx(first)) { - Ok(c) => c, - Err(e) => { - match (e, second) { - (LayoutError::NotEnoughSpace(_), Some(space)) => { - func.body.val.layout(ctx(space))? - } - (e, _) => Err(e)?, - } - } - }; + let commands = func.body.val.layout(LayoutContext { + loader: self.ctx.loader, + top_level: false, + text_style: &self.style, + page_style: self.ctx.page_style, + spaces, + axes, + expand: false, + })?; for command in commands { self.execute(command)?; @@ -123,7 +109,7 @@ impl<'a, 'p> TreeLayouter<'a, 'p> { Command::SetTextStyle(style) => self.style = style, Command::SetPageStyle(style) => { if !self.ctx.top_level { - Err(LayoutError::Unallowed("can only set page style from top level"))?; + lerr!("page style cannot only be altered in the top-level context"); } self.ctx.page_style = style; diff --git a/src/lib.rs b/src/lib.rs index a5551c778..b1408f4a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -133,8 +133,8 @@ pub enum TypesetError { error_type! { err: TypesetError, show: f => match err { - TypesetError::Parse(e) => write!(f, "parse error: {}", e), - TypesetError::Layout(e) => write!(f, "layout error: {}", e), + TypesetError::Parse(e) => write!(f, "{}", e), + TypesetError::Layout(e) => write!(f, "{}", e), }, source: match err { TypesetError::Parse(e) => Some(e), diff --git a/src/library/axes.rs b/src/library/align.rs similarity index 92% rename from src/library/axes.rs rename to src/library/align.rs index 29b624abd..55063ebe3 100644 --- a/src/library/axes.rs +++ b/src/library/align.rs @@ -52,7 +52,7 @@ function! { if target.is_none() { *target = Some(parse_align_specifier(arg)?); } else { - err!("duplicate alignment specification for {} axis", axis); + perr!("duplicate alignment specification for {} axis", axis); } }) }; @@ -136,7 +136,7 @@ fn parse_align_specifier(arg: Spanned<&str>) -> ParseResult { "right" => AlignSpecifier::Right, "top" => AlignSpecifier::Top, "bottom" => AlignSpecifier::Bottom, - s => err!("invalid alignment specifier: {}", s), + s => perr!("invalid alignment specifier: {}", s), }) } @@ -146,6 +146,10 @@ fn generic_alignment(spec: AlignSpecifier, horizontal: bool) -> LayoutResult Alignment::Origin, (Center, _) => Alignment::Center, (End, _) | (Right, true) | (Bottom, false) => Alignment::End, - _ => Err(LayoutError::UnalignedAxis("invalid alignment"))?, + _ => lerr!( + "invalid alignment specifier `{}` for {} axis", + format!("{:?}", spec).to_lowercase(), + if horizontal { "horizontal" } else { "vertical" }, + ), }) } diff --git a/src/library/mod.rs b/src/library/mod.rs index 5fa326c89..adcb974b1 100644 --- a/src/library/mod.rs +++ b/src/library/mod.rs @@ -3,7 +3,7 @@ use crate::func::Scope; pub_use_mod!(boxed); -pub_use_mod!(axes); +pub_use_mod!(align); pub_use_mod!(spacing); pub_use_mod!(style); pub_use_mod!(page); diff --git a/src/library/spacing.rs b/src/library/spacing.rs index 010594847..d52153b1a 100644 --- a/src/library/spacing.rs +++ b/src/library/spacing.rs @@ -38,7 +38,7 @@ macro_rules! space_func { let spacing = match arg.val { Expression::Size(s) => Spacing::Absolute(*s), Expression::Num(f) => Spacing::Relative(*f as f32), - _ => err!("invalid spacing, expected size or number"), + _ => perr!("invalid spacing, expected size or number"), }; Ok($ident(spacing)) diff --git a/src/syntax/mod.rs b/src/syntax/mod.rs index 09a5bdc39..9406fdf40 100644 --- a/src/syntax/mod.rs +++ b/src/syntax/mod.rs @@ -6,6 +6,7 @@ use crate::func::Function; use crate::size::Size; mod tokens; +#[macro_use] mod parsing; pub use tokens::{tokenize, Tokens}; diff --git a/src/syntax/parsing.rs b/src/syntax/parsing.rs index e4cda6a59..b3fda7344 100644 --- a/src/syntax/parsing.rs +++ b/src/syntax/parsing.rs @@ -120,10 +120,10 @@ impl<'s> Parser<'s> { if is_identifier(word) { Ok(Spanned::new(word.to_owned(), span)) } else { - err!("invalid identifier: '{}'", word); + perr!("invalid identifier: '{}'", word); } } - _ => err!("expected identifier"), + _ => perr!("expected identifier"), }?; self.skip_white(); @@ -132,7 +132,7 @@ impl<'s> Parser<'s> { let args = match self.tokens.next().map(Spanned::value) { Some(Token::RightBracket) => FuncArgs::new(), Some(Token::Colon) => self.parse_func_args()?, - _ => err!("expected arguments or closing bracket"), + _ => perr!("expected arguments or closing bracket"), }; let end = self.tokens.string_index(); @@ -158,7 +158,7 @@ impl<'s> Parser<'s> { match self.tokens.next().map(Spanned::value) { Some(Token::Comma) => {}, Some(Token::RightBracket) => break, - _ => err!("expected comma or closing bracket"), + _ => perr!("expected comma or closing bracket"), } } @@ -183,7 +183,7 @@ impl<'s> Parser<'s> { self.skip_white(); let name = token.span_map(|_| name.to_string()); - let next = self.tokens.next().ok_or_else(|| err!(@"expected value"))?; + let next = self.tokens.next().ok_or_else(|| perr!(@"expected value"))?; let val = Self::parse_expression(next)?; let span = Span::merge(name.span, val.span); @@ -219,7 +219,7 @@ impl<'s> Parser<'s> { } } - _ => err!("expected expression"), + _ => perr!("expected expression"), }, token.span)) } @@ -230,7 +230,7 @@ impl<'s> Parser<'s> { .ctx .scope .get_parser(&header.name.val) - .ok_or_else(|| err!(@"unknown function: '{}'", &header.name.val))?; + .ok_or_else(|| perr!(@"unknown function: '{}'", &header.name.val))?; let has_body = self.tokens.peek().map(Spanned::value) == Some(Token::LeftBracket); @@ -298,7 +298,7 @@ impl<'s> Parser<'s> { state = NewlineState::Zero; match token.val { Token::LineComment(_) | Token::BlockComment(_) => self.advance(), - Token::StarSlash => err!("unexpected end of block comment"), + Token::StarSlash => perr!("unexpected end of block comment"), _ => break, } } @@ -448,6 +448,7 @@ error_type! { show: f => f.write_str(&err.0), } + #[cfg(test)] mod tests { #![allow(non_snake_case)] diff --git a/tests/layouting.rs b/tests/layouting.rs index 77ec54ae1..72d160e41 100644 --- a/tests/layouting.rs +++ b/tests/layouting.rs @@ -68,32 +68,39 @@ fn test(name: &str, src: &str) { let provider = FileSystemFontProvider::from_listing("fonts/fonts.toml").unwrap(); typesetter.add_font_provider(provider.clone()); - #[cfg(not(debug_assertions))] - let layouts = { + #[cfg(not(debug_assertions))] { use std::time::Instant; // Warmup. let warmup_start = Instant::now(); - typesetter.typeset(&src).unwrap(); + let is_ok = typesetter.typeset(&src).is_ok(); let warmup_end = Instant::now(); - let start = Instant::now(); - let tree = typesetter.parse(&src).unwrap(); - let mid = Instant::now(); - let layouts = typesetter.layout(&tree).unwrap(); - let end = Instant::now(); + if is_ok { + let start = Instant::now(); + let tree = typesetter.parse(&src).unwrap(); + let mid = Instant::now(); + typesetter.layout(&tree).unwrap(); + let end = Instant::now(); - println!(" - cold start: {:?}", warmup_end - warmup_start); - println!(" - warmed up: {:?}", end - start); - println!(" - parsing: {:?}", mid - start); - println!(" - layouting: {:?}", end - mid); - println!(); - - layouts + println!(" - cold start: {:?}", warmup_end - warmup_start); + println!(" - warmed up: {:?}", end - start); + println!(" - parsing: {:?}", mid - start); + println!(" - layouting: {:?}", end - mid); + println!(); + } + }; + + let layouts = match typesetter.typeset(&src) { + Ok(layouts) => layouts, + Err(err) => { + println!(" - compilation failed: {}", err); + #[cfg(not(debug_assertions))] + println!(); + return; + }, }; - #[cfg(debug_assertions)] - let layouts = typesetter.typeset(&src).unwrap(); // Write the serialed layout file. let path = format!("{}/serialized/{}.tld", CACHE_DIR, name);