From 9a4e8357e7326299d098632f6efa8bcde7f0ca58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Cicho=C5=84?= Date: Sat, 13 Jan 2024 18:58:20 +0100 Subject: [PATCH] deps: add thiserror and anyhow, reduce boilerplate This adds rudimentary error messages and unsilences some errors. --- Cargo.toml | 2 ++ src/draw.rs | 19 +++++++------- src/layout.rs | 69 +++++++++++++++++++-------------------------------- src/main.rs | 12 +++++---- src/math.rs | 4 ++- src/router.rs | 35 +++++++++++++++++++++++--- 6 files changed, 78 insertions(+), 63 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 43fc8f5..9b6ece9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,8 @@ pathfinder_geometry = { git = "https://github.com/servo/pathfinder" } pathfinder_gl = { git = "https://github.com/servo/pathfinder" } pathfinder_renderer = { git = "https://github.com/servo/pathfinder" } pathfinder_resources = { git = "https://github.com/servo/pathfinder" } +thiserror = "1.0.56" +anyhow = "1.0.79" [dependencies.geo] version = "0.25.1" diff --git a/src/draw.rs b/src/draw.rs index 6bc7811..50d38a2 100644 --- a/src/draw.rs +++ b/src/draw.rs @@ -1,6 +1,6 @@ use contracts::debug_ensures; - use geo::{EuclideanLength, Point}; +use thiserror::Error; use crate::{ graph::{ @@ -15,19 +15,18 @@ use crate::{ rules::{Conditions, Rules}, }; -#[derive(Debug, Clone, Copy)] +#[derive(Error, Debug, Clone, Copy)] pub enum DrawException { - NoTangents(NoTangents), - CannotFinishIn(FixedDotIndex, LayoutException), + #[error(transparent)] + NoTangents(#[from] NoTangents), + // TODO add real error messages + these should eventually use Display + #[error("cannot finish in {0:?}")] + CannotFinishIn(FixedDotIndex, #[source] LayoutException), + #[error("cannot wrap around {0:?}")] + // neither of the exceptions is the source on its own, might be useful to give them names? CannotWrapAround(WraparoundableIndex, LayoutException, LayoutException), } -impl From for DrawException { - fn from(err: NoTangents) -> Self { - DrawException::NoTangents(err) - } -} - pub struct Draw<'a> { layout: &'a mut Layout, rules: &'a Rules, diff --git a/src/layout.rs b/src/layout.rs index 4e621bf..8b4b103 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -7,6 +7,7 @@ use petgraph::Direction::Incoming; use rstar::primitives::GeomWithData; use rstar::{RTree, RTreeObject}; use slab::Slab; +use thiserror::Error; use crate::graph::{ BendWeight, DotIndex, DotWeight, FixedBendIndex, FixedDotIndex, FixedDotWeight, FixedSegIndex, @@ -26,46 +27,30 @@ use crate::shape::{Shape, ShapeTrait}; pub type RTreeWrapper = GeomWithData; #[enum_dispatch] -#[derive(Debug, Clone, Copy)] +#[derive(Error, Debug, Clone, Copy)] pub enum LayoutException { - NoTangents(NoTangents), - Infringement(Infringement), - Collision(Collision), - IsConnected(IsConnected), + #[error(transparent)] + NoTangents(#[from] NoTangents), + #[error(transparent)] + Infringement(#[from] Infringement), + #[error(transparent)] + Collision(#[from] Collision), + #[error(transparent)] + AlreadyConnected(#[from] AlreadyConnected), } -impl From for LayoutException { - fn from(err: NoTangents) -> Self { - LayoutException::NoTangents(err) - } -} - -impl From for LayoutException { - fn from(err: Infringement) -> Self { - LayoutException::Infringement(err) - } -} - -impl From for LayoutException { - fn from(err: Collision) -> Self { - LayoutException::Collision(err) - } -} - -impl From for LayoutException { - fn from(err: IsConnected) -> Self { - LayoutException::IsConnected(err) - } -} - -#[derive(Debug, Clone, Copy)] +// TODO add real error messages + these should eventually use Display +#[derive(Error, Debug, Clone, Copy)] +#[error("{0:?} infringes on {1:?}")] pub struct Infringement(pub Shape, pub Index); -#[derive(Debug, Clone, Copy)] +#[derive(Error, Debug, Clone, Copy)] +#[error("{0:?} collides with {1:?}")] pub struct Collision(pub Shape, pub Index); -#[derive(Debug, Clone, Copy)] -pub struct IsConnected(pub i64, pub Index); +#[derive(Error, Debug, Clone, Copy)] +#[error("{1:?} is already connected to net {0}")] +pub struct AlreadyConnected(pub i64, pub Index); #[derive(Debug, Clone, Copy)] pub struct Band { @@ -152,7 +137,7 @@ impl Layout { self.insert_into_rtree(dot.into()); self.fail_and_remove_if_infringes_except(dot.into(), infringables)?; - Ok::, Infringement>(dot) + Ok(dot) } #[debug_ensures(ret.is_ok() -> self.graph.node_count() == old(self.graph.node_count() + 1))] @@ -471,8 +456,7 @@ impl Layout { .map_err(|err| { self.remove(seg_to.into()); err - }) - .map_err(|err| LayoutException::Infringement(err))?; + })?; let bend_to = self .add_dot_infringably(dot_weight, infringables) @@ -480,8 +464,7 @@ impl Layout { self.remove(seg.into()); self.remove(seg_to.into()); err - }) - .map_err(|err| LayoutException::Infringement(err))?; + })?; let bend = self .add_loose_bend_infringably(seg_to, bend_to, around, bend_weight, infringables) .map_err(|err| { @@ -571,10 +554,10 @@ impl Layout { let net = self.bands[weight.band].net; // if net == around.primitive(self).net() { - return Err(LayoutException::IsConnected(IsConnected( + return Err(AlreadyConnected( net, around.into(), - ))); + ).into()); } // if let Some(wraparound) = match around { @@ -583,10 +566,10 @@ impl Layout { WraparoundableIndex::LooseBend(around) => self.primitive(around).wraparound(), } { if net == wraparound.primitive(self).net() { - return Err(LayoutException::IsConnected(IsConnected( + return Err(AlreadyConnected( net, wraparound.into(), - ))); + ).into()); } } @@ -673,7 +656,7 @@ impl Layout { self.insert_into_rtree(bend.into()); self.fail_and_remove_if_infringes_except(bend.into(), infringables)?; - Ok::(bend) + Ok(bend) } #[debug_ensures(self.graph.node_count() == old(self.graph.node_count()))] diff --git a/src/main.rs b/src/main.rs index 61d4e3a..c67a099 100644 --- a/src/main.rs +++ b/src/main.rs @@ -31,7 +31,7 @@ use layout::{Infringement, Layout, LayoutException}; use mesh::{Mesh, MeshEdgeReference, VertexIndex}; use petgraph::visit::{EdgeRef, IntoEdgeReferences}; use primitive::MakeShape; -use router::RouterObserver; +use router::{RouterObserver, RoutingError}; use sdl2::event::Event; use sdl2::keyboard::Keycode; @@ -175,7 +175,7 @@ impl<'a> RouterObserver for DebugRouterObserver<'a> { fn on_estimate(&mut self, _tracer: &Tracer, _vertex: VertexIndex) {} } -fn main() { +fn main() -> Result<(), anyhow::Error> { let sdl_context = sdl2::init().unwrap(); let video_subsystem = sdl_context.video().unwrap(); @@ -488,7 +488,7 @@ fn main() { dot_end, &mut EmptyRouterObserver, //&mut DebugRouterObserver::new(&mut event_pump, &window, &mut renderer, &font_context), - ); + )?; render_times( &mut event_pump, @@ -521,7 +521,7 @@ fn main() { dot_end2, //&mut EmptyRouterObserver, &mut DebugRouterObserver::new(&mut event_pump, &window, &mut renderer, &font_context), - ); + )?; render_times( &mut event_pump, @@ -542,7 +542,7 @@ fn main() { dot_start3, dot_end3, &mut DebugRouterObserver::new(&mut event_pump, &window, &mut renderer, &font_context), - ); + )?; render_times( &mut event_pump, @@ -558,6 +558,8 @@ fn main() { &[], -1, ); + + Ok(()) } fn render_times( diff --git a/src/math.rs b/src/math.rs index 54b03dd..76431d9 100644 --- a/src/math.rs +++ b/src/math.rs @@ -1,7 +1,9 @@ use geo::{geometry::Point, point, EuclideanDistance, Line}; use std::ops::Sub; +use thiserror::Error; -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Error, Debug, Clone, Copy, PartialEq)] +#[error("no tangents for {0:?} and {1:?}")] // TODO add real error message pub struct NoTangents(pub Circle, pub Circle); #[derive(Debug, Clone, Copy, PartialEq)] diff --git a/src/router.rs b/src/router.rs index f8c6e3b..fcdd9d9 100644 --- a/src/router.rs +++ b/src/router.rs @@ -1,6 +1,7 @@ use geo::geometry::Point; use petgraph::visit::EdgeRef; use spade::InsertionError; +use thiserror::Error; use crate::astar::{astar, AstarStrategy, PathTracker}; use crate::draw::DrawException; @@ -12,6 +13,24 @@ use crate::mesh::{Mesh, MeshEdgeReference, VertexIndex}; use crate::rules::Rules; use crate::tracer::{Trace, Tracer}; +#[derive(Error, Debug, Clone, Copy)] +#[error("failed to route from {from:?} to {to:?}")] // this should eventually use Display +pub struct RoutingError { + from: FixedDotIndex, + to: FixedDotIndex, + source: RoutingErrorKind, +} + +#[derive(Error, Debug, Clone, Copy)] +pub enum RoutingErrorKind { + #[error(transparent)] + MeshInsertion(#[from] InsertionError), + // exposing more details here seems difficult + // TODO more descriptive message + #[error("A* found no path")] + AStar, +} + pub trait RouterObserver { fn on_rework(&mut self, tracer: &Tracer, trace: &Trace); fn before_probe(&mut self, tracer: &Tracer, trace: &Trace, edge: MeshEdgeReference); @@ -97,12 +116,17 @@ impl Router { from: FixedDotIndex, to: FixedDotIndex, observer: &mut impl RouterObserver, - ) -> Result { + ) -> Result { // XXX: Should we actually store the mesh? May be useful for debugging, but doesn't look // right. //self.mesh.triangulate(&self.layout)?; let mut mesh = Mesh::new(&self.layout); - mesh.generate(&self.layout)?; + mesh.generate(&self.layout) + .map_err(|err| RoutingError { + from, + to, + source: err.into(), + })?; let mut tracer = self.tracer(&mesh); let trace = tracer.start(from, 3.0); @@ -111,8 +135,11 @@ impl Router { &mesh, from.into(), &mut RouterAstarStrategy::new(tracer, trace, to.into(), observer), - ) - .unwrap(); // TODO. + ).ok_or(RoutingError { + from, + to, + source: RoutingErrorKind::AStar, + })?; Ok(mesh) }