From 825a36752514d400ada664febf19122688131181 Mon Sep 17 00:00:00 2001 From: Andrey Zgarbul Date: Thu, 14 Nov 2019 21:40:29 +0300 Subject: [PATCH] anyhow & forward errors instead of unwrap() --- CHANGELOG.md | 2 + Cargo.toml | 3 +- src/errors.rs | 1 - src/generate/device.rs | 8 +-- src/generate/interrupt.rs | 4 +- src/generate/peripheral.rs | 111 ++++++++++++++++++------------------- src/generate/register.rs | 70 +++++++++++++---------- src/lib.rs | 11 ++-- src/main.rs | 29 +++------- src/util.rs | 22 +++----- 10 files changed, 126 insertions(+), 135 deletions(-) delete mode 100644 src/errors.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b5030f4..faf80908 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Changed +- `anyhow` crate is used for error handling + - [breaking-change] Among other cleanups, MSP430 crates are now expected to use the `msp430_rt::interrupt` attribute macro and `device.x` for interrupt support. The `INTERRUPT` array has been renamed `__INTERRUPT`. diff --git a/Cargo.toml b/Cargo.toml index d1831106..b498a932 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,11 +35,12 @@ path = "src/main.rs" cast = "0.2" clap = "2.33" env_logger = "~0.7" -error-chain = "0.12" inflections = "1.1" log = { version = "~0.4", features = ["std"] } quote = "1.0" proc-macro2 = "1.0" +anyhow = "1.0.19" +thiserror = "1.0.5" [dependencies.svd-parser] version = "0.9" diff --git a/src/errors.rs b/src/errors.rs deleted file mode 100644 index c0444737..00000000 --- a/src/errors.rs +++ /dev/null @@ -1 +0,0 @@ -error_chain!(); diff --git a/src/generate/device.rs b/src/generate/device.rs index 3794ea99..eb24cd92 100644 --- a/src/generate/device.rs +++ b/src/generate/device.rs @@ -4,9 +4,9 @@ use quote::ToTokens; use std::fs::File; use std::io::Write; -use crate::errors::*; use crate::util::{self, ToSanitizedUpperCase}; use crate::Target; +use anyhow::Result; use crate::generate::{interrupt, peripheral}; @@ -155,11 +155,11 @@ pub fn render( }); } - let generic_file = std::str::from_utf8(include_bytes!("generic.rs")).unwrap(); + let generic_file = std::str::from_utf8(include_bytes!("generic.rs"))?; if generic_mod { - writeln!(File::create("generic.rs").unwrap(), "{}", generic_file).unwrap(); + writeln!(File::create("generic.rs")?, "{}", generic_file)?; } else { - let tokens = syn::parse_file(generic_file).unwrap().into_token_stream(); + let tokens = syn::parse_file(generic_file)?.into_token_stream(); out.extend(quote! { #[allow(unused_imports)] diff --git a/src/generate/interrupt.rs b/src/generate/interrupt.rs index 20f65cc6..56be1826 100644 --- a/src/generate/interrupt.rs +++ b/src/generate/interrupt.rs @@ -5,9 +5,9 @@ use crate::svd::Peripheral; use cast::u64; use proc_macro2::{Ident, Span, TokenStream}; -use crate::errors::*; use crate::util::{self, ToSanitizedUpperCase}; use crate::Target; +use anyhow::Result; /// Generates code for `src/interrupt.rs` pub fn render( @@ -72,7 +72,7 @@ pub fn render( match target { Target::CortexM => { for name in &names { - writeln!(device_x, "PROVIDE({} = DefaultHandler);", name).unwrap(); + writeln!(device_x, "PROVIDE({} = DefaultHandler);", name)?; } root.extend(quote! { diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs index bd41cccf..feffb6a4 100644 --- a/src/generate/peripheral.rs +++ b/src/generate/peripheral.rs @@ -10,8 +10,8 @@ use quote::ToTokens; use svd_parser::derive_from::DeriveFrom; use syn::{parse_str, Token}; -use crate::errors::*; use crate::util::{self, ToSanitizedSnakeCase, ToSanitizedUpperCase, BITS_PER_BYTE}; +use anyhow::{anyhow, bail, Context, Result}; use crate::generate::register; @@ -31,11 +31,10 @@ pub fn render( let p_merged = p_derivedfrom.map(|ancestor| p_original.derive_from(ancestor)); let p = p_merged.as_ref().unwrap_or(p_original); - if p_original.derived_from.is_some() && p_derivedfrom.is_none() { + if let (Some(df), None) = (p_original.derived_from.as_ref(), &p_derivedfrom) { eprintln!( "Couldn't find derivedFrom original: {} for {}, skipping", - p_original.derived_from.as_ref().unwrap(), - p_original.name + df, p_original.name ); return Ok(out); } @@ -44,13 +43,12 @@ pub fn render( let name_pc = Ident::new(&p.name.to_sanitized_upper_case(), span); let address = util::hex(p.base_address as u64); let description = util::respace(p.description.as_ref().unwrap_or(&p.name)); - let derive_regs = p_derivedfrom.is_some() && p_original.registers.is_none(); let name_sc = Ident::new(&p.name.to_sanitized_snake_case(), span); - let base = if derive_regs { - Ident::new(&p_derivedfrom.unwrap().name.to_sanitized_snake_case(), span) + let (derive_regs, base) = if let (Some(df), None) = (p_derivedfrom, &p_original.registers) { + (true, Ident::new(&df.name.to_sanitized_snake_case(), span)) } else { - name_sc.clone() + (false, name_sc.clone()) }; // Insert the peripheral structure @@ -607,11 +605,11 @@ fn expand_cluster(cluster: &Cluster, defs: &RegisterProperties) -> Result cluster_expanded.push(RegisterBlockField { - field: convert_svd_cluster(cluster), + field: convert_svd_cluster(cluster)?, description: info.description.as_ref().unwrap_or(&info.name).into(), offset: info.address_offset, size: cluster_size, @@ -631,13 +629,13 @@ fn expand_cluster(cluster: &Cluster, defs: &RegisterProperties) -> Result register_expanded.push(RegisterBlockField { - field: convert_svd_register(register, name), - description: info.description.clone().unwrap_or_else(|| "".to_string()), + field: convert_svd_register(register, name)?, + description: info.description.clone().unwrap_or_default(), offset: info.address_offset, size: register_size, }), @@ -688,16 +686,16 @@ fn expand_register( if array_convertible { register_expanded.push(RegisterBlockField { - field: convert_svd_register(®ister, name), - description: info.description.clone().unwrap_or_else(|| "".to_string()), + field: convert_svd_register(®ister, name)?, + description: info.description.clone().unwrap_or_default(), offset: info.address_offset, size: register_size * array_info.dim, }); } else { - for (field_num, field) in expand_svd_register(register, name).iter().enumerate() { + for (field_num, field) in expand_svd_register(register, name)?.iter().enumerate() { register_expanded.push(RegisterBlockField { field: field.clone(), - description: info.description.clone().unwrap_or_else(|| "".to_string()), + description: info.description.clone().unwrap_or_default(), offset: info.address_offset + field_num as u32 * array_info.dim_increment, size: register_size, }); @@ -768,8 +766,11 @@ fn cluster_block( /// Takes a svd::Register which may be a register array, and turn in into /// a list of syn::Field where the register arrays have been expanded. -fn expand_svd_register(register: &Register, name: Option<&str>) -> Vec { - let name_to_ty = |name: &String, ns: Option<&str>| -> syn::Type { +fn expand_svd_register( + register: &Register, + name: Option<&str>, +) -> Result, syn::Error> { + let name_to_ty = |name: &String, ns: Option<&str>| -> Result { let ident = if let Some(ns) = ns { Cow::Owned( String::from("self::") @@ -781,13 +782,13 @@ fn expand_svd_register(register: &Register, name: Option<&str>) -> Vec(&ident).unwrap()) + Ok(syn::Type::Path(parse_str::(&ident)?)) }; let mut out = vec![]; match register { - Register::Single(_info) => out.push(convert_svd_register(register, name)), + Register::Single(_info) => out.push(convert_svd_register(register, name)?), Register::Array(info, array_info) => { let indices = array_info .dim_index @@ -806,17 +807,17 @@ fn expand_svd_register(register: &Register, name: Option<&str>) -> Vec) -> syn::Field { +fn convert_svd_register(register: &Register, name: Option<&str>) -> Result { let name_to_ty = |name: &String, ns: Option<&str>| -> String { if let Some(ns) = ns { String::from("self::") @@ -828,39 +829,38 @@ fn convert_svd_register(register: &Register, name: Option<&str>) -> syn::Field { } }; - match register { + Ok(match register { Register::Single(info) => new_syn_field( &info.name.to_sanitized_snake_case(), - syn::Type::Path(parse_str::(&name_to_ty(&info.name, name)).unwrap()), + syn::Type::Path(parse_str::(&name_to_ty(&info.name, name))?), ), Register::Array(info, array_info) => { let nb_name = util::replace_suffix(&info.name, ""); - let ty = syn::Type::Array( - parse_str::(&format!( - "[{};{}]", - name_to_ty(&nb_name, name), - u64::from(array_info.dim) - )) - .unwrap(), - ); + let ty = syn::Type::Array(parse_str::(&format!( + "[{};{}]", + name_to_ty(&nb_name, name), + u64::from(array_info.dim) + ))?); new_syn_field(&nb_name.to_sanitized_snake_case(), ty) } - } + }) } /// Takes a svd::Cluster which may contain a register array, and turn in into /// a list of syn::Field where the register arrays have been expanded. -fn expand_svd_cluster(cluster: &Cluster) -> Vec { - let name_to_ty = |name: &String| -> syn::Type { - syn::Type::Path(parse_str::(&name.to_sanitized_upper_case()).unwrap()) +fn expand_svd_cluster(cluster: &Cluster) -> Result, syn::Error> { + let name_to_ty = |name: &String| -> Result { + Ok(syn::Type::Path(parse_str::( + &name.to_sanitized_upper_case(), + )?)) }; let mut out = vec![]; match &cluster { - Cluster::Single(_info) => out.push(convert_svd_cluster(cluster)), + Cluster::Single(_info) => out.push(convert_svd_cluster(cluster)?), Cluster::Array(info, array_info) => { let indices = array_info .dim_index @@ -879,39 +879,36 @@ fn expand_svd_cluster(cluster: &Cluster) -> Vec { for (idx, _i) in indices.iter().zip(0..) { let name = util::replace_suffix(&info.name, idx); - let ty = name_to_ty(&ty_name); + let ty = name_to_ty(&ty_name)?; out.push(new_syn_field(&name.to_sanitized_snake_case(), ty)); } } } - out + Ok(out) } /// Convert a parsed `Cluster` into its `Field` equivalent -fn convert_svd_cluster(cluster: &Cluster) -> syn::Field { - match cluster { +fn convert_svd_cluster(cluster: &Cluster) -> Result { + Ok(match cluster { Cluster::Single(info) => new_syn_field( &info.name.to_sanitized_snake_case(), - syn::Type::Path( - parse_str::(&info.name.to_sanitized_upper_case()).unwrap(), - ), + syn::Type::Path(parse_str::( + &info.name.to_sanitized_upper_case(), + )?), ), Cluster::Array(info, array_info) => { let name = util::replace_suffix(&info.name, ""); - let ty = syn::Type::Array( - parse_str::(&format!( - "[{};{}]", - &name.to_sanitized_upper_case(), - u64::from(array_info.dim) - )) - .unwrap(), - ); + let ty = syn::Type::Array(parse_str::(&format!( + "[{};{}]", + &name.to_sanitized_upper_case(), + u64::from(array_info.dim) + ))?); new_syn_field(&name.to_sanitized_snake_case(), ty) } - } + }) } fn new_syn_field(ident: &str, ty: syn::Type) -> syn::Field { diff --git a/src/generate/register.rs b/src/generate/register.rs index 53749f0f..148714bd 100644 --- a/src/generate/register.rs +++ b/src/generate/register.rs @@ -7,8 +7,8 @@ use cast::u64; use log::warn; use proc_macro2::{Ident, Punct, Spacing, Span, TokenStream}; -use crate::errors::*; use crate::util::{self, ToSanitizedSnakeCase, ToSanitizedUpperCase, U32Ext}; +use anyhow::{anyhow, Result}; pub fn render( register: &Register, @@ -26,7 +26,7 @@ pub fn render( let rsize = register .size .or(defs.size) - .ok_or_else(|| format!("Register {} has no `size` field", register.name))?; + .ok_or_else(|| anyhow!("Register {} has no `size` field", register.name))?; let rsize = if rsize < 8 { 8 } else if rsize.is_power_of_two() { @@ -38,7 +38,7 @@ pub fn render( let description = util::escape_brackets( util::respace(®ister.description.clone().unwrap_or_else(|| { warn!("Missing description for register {}", register.name); - "".to_string() + Default::default() })) .as_ref(), ); @@ -264,7 +264,7 @@ pub fn fields( .map(|element| element.parse::()) .eq((first..de.dim + first).map(Ok)); if !sequential_indexes { - return Err(format!("unsupported array indexes in {}", f.name))?; + return Err(anyhow!("unsupported array indexes in {}", f.name))?; } (first, None) } else { @@ -282,7 +282,7 @@ pub fn fields( } Field::Single(_) => { if f.name.contains("%s") { - return Err(format!("incorrect field {}", f.name))?; + return Err(anyhow!("incorrect field {}", f.name))?; } None } @@ -678,7 +678,7 @@ impl Variant { .filter(|field| field.name.to_lowercase() != "reserved") .map(|ev| { let value = u64(ev.value.ok_or_else(|| { - format!("EnumeratedValue {} has no `` field", ev.name) + anyhow!("EnumeratedValue {} has no `` field", ev.name) })?); Ok(Variant { @@ -896,9 +896,10 @@ fn lookup_in_fields<'f>( if let Some(base_field) = fields.iter().find(|f| f.name == base_field) { lookup_in_field(base_evs, None, None, base_field) } else { - Err(format!( + Err(anyhow!( "Field {} not found in register {}", - base_field, register.name + base_field, + register.name ) .into()) } @@ -923,12 +924,17 @@ fn lookup_in_peripheral<'p>( { lookup_in_field(base_evs, Some(base_register), base_peripheral, field) } else { - Err(format!("No field {} in register {}", base_field, register.name).into()) + Err(anyhow!( + "No field {} in register {}", + base_field, + register.name + ))? } } else { - Err(format!( + Err(anyhow!( "No register {} in peripheral {}", - base_register, peripheral.name + base_register, + peripheral.name ) .into()) } @@ -953,7 +959,11 @@ fn lookup_in_field<'f>( } } - Err(format!("No EnumeratedValues {} in field {}", base_evs, field.name).into()) + Err(anyhow!( + "No EnumeratedValues {} in field {}", + base_evs, + field.name + ))? } fn lookup_in_register<'r>( @@ -973,9 +983,10 @@ fn lookup_in_register<'r>( } match matches.first() { - None => Err(format!( + None => Err(anyhow!( "EnumeratedValues {} not found in register {}", - base_evs, register.name + base_evs, + register.name ) .into()), Some(&(evs, field)) => { @@ -990,10 +1001,11 @@ fn lookup_in_register<'r>( )) } else { let fields = matches.iter().map(|(f, _)| &f.name).collect::>(); - Err(format!( + Err(anyhow!( "Fields {:?} have an \ enumeratedValues named {}", - fields, base_evs + fields, + base_evs ) .into()) } @@ -1019,7 +1031,7 @@ fn lookup_in_peripherals<'p>( peripheral, ) } else { - Err(format!("No peripheral {}", base_peripheral).into()) + Err(anyhow!("No peripheral {}", base_peripheral))? } } @@ -1037,21 +1049,19 @@ fn periph_all_registers<'a>(p: &'a Peripheral) -> Vec<&'a Register> { } loop { - let b = rem.pop(); - if b.is_none() { - break; - } - - let b = b.unwrap(); - match b { - RegisterCluster::Register(reg) => { - par.push(reg); - } - RegisterCluster::Cluster(cluster) => { - for c in cluster.children.iter() { - rem.push(c); + if let Some(b) = rem.pop() { + match b { + RegisterCluster::Register(reg) => { + par.push(reg); + } + RegisterCluster::Cluster(cluster) => { + for c in cluster.children.iter() { + rem.push(c); + } } } + } else { + break; } } par diff --git a/src/lib.rs b/src/lib.rs index 4c013f98..ed5c0949 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -481,13 +481,10 @@ //! compiler. Currently there are no nightly features the flag is only kept for compatibility with prior versions. #![recursion_limit = "128"] -#[macro_use] -extern crate error_chain; #[macro_use] extern crate quote; use svd_parser as svd; -mod errors; mod generate; mod util; @@ -509,10 +506,12 @@ pub struct DeviceSpecific { _extensible: (), } -type Result = std::result::Result; -#[derive(Debug)] +use anyhow::Result; +#[derive(Debug, thiserror::Error)] pub enum SvdError { + #[error("Cannot format crate")] Fmt, + #[error("Cannot render SVD device")] Render, } @@ -520,7 +519,7 @@ pub enum SvdError { pub fn generate(xml: &str, target: Target, nightly: bool) -> Result { use std::fmt::Write; - let device = svd::parse(xml).unwrap(); //TODO(AJM) + let device = svd::parse(xml)?; let mut device_x = String::new(); let items = generate::device::render(&device, target, nightly, false, &mut device_x) .or(Err(SvdError::Render))?; diff --git a/src/main.rs b/src/main.rs index 20237961..b0afc532 100755 --- a/src/main.rs +++ b/src/main.rs @@ -1,14 +1,11 @@ #![recursion_limit = "128"] -#[macro_use] -extern crate error_chain; #[macro_use] extern crate log; #[macro_use] extern crate quote; use svd_parser as svd; -mod errors; mod generate; mod util; @@ -16,9 +13,9 @@ use std::fs::File; use std::io::Write; use std::process; +use anyhow::{Context, Result}; use clap::{App, Arg}; -use crate::errors::*; use crate::util::{build_rs, Target}; fn run() -> Result<()> { @@ -79,20 +76,20 @@ fn run() -> Result<()> { match matches.value_of("input") { Some(file) => { File::open(file) - .chain_err(|| "couldn't open the SVD file")? + .context("Cannot open the SVD file")? .read_to_string(xml) - .chain_err(|| "couldn't read the SVD file")?; + .context("Cannot read the SVD file")?; } None => { let stdin = std::io::stdin(); stdin .lock() .read_to_string(xml) - .chain_err(|| "couldn't read from stdin")?; + .context("Cannot read from stdin")?; } } - let device = svd::parse(xml).unwrap(); //TODO(AJM) + let device = svd::parse(xml)?; let nightly = matches.is_present("nightly_features"); @@ -107,8 +104,8 @@ fn run() -> Result<()> { .expect("Could not write code to lib.rs"); if target == Target::CortexM || target == Target::Msp430 { - writeln!(File::create("device.x").unwrap(), "{}", device_x).unwrap(); - writeln!(File::create("build.rs").unwrap(), "{}", build_rs()).unwrap(); + writeln!(File::create("device.x")?, "{}", device_x)?; + writeln!(File::create("build.rs")?, "{}", build_rs())?; } Ok(()) @@ -142,17 +139,7 @@ fn setup_logging(matches: &clap::ArgMatches) { fn main() { if let Err(ref e) = run() { - error!("{}", e); - - for e in e.iter().skip(1) { - error!("caused by: {}", e); - } - - if let Some(backtrace) = e.backtrace() { - error!("backtrace: {:?}", backtrace); - } else { - error!("note: run with `RUST_BACKTRACE=1` for a backtrace") - } + error!("{:?}", e); process::exit(1); } diff --git a/src/util.rs b/src/util.rs index ffcc3e32..c4fa58b4 100644 --- a/src/util.rs +++ b/src/util.rs @@ -5,7 +5,7 @@ use crate::svd::{Access, Cluster, Register, RegisterCluster}; use inflections::Inflect; use proc_macro2::{Ident, Literal, Span, TokenStream}; -use crate::errors::*; +use anyhow::{anyhow, bail, Result}; pub const BITS_PER_BYTE: u32 = 8; @@ -277,11 +277,10 @@ impl U32Ext for u32 { 9..=16 => "u16", 17..=32 => "u32", 33..=64 => "u64", - _ => { - return Err( - format!("can't convert {} bits into a Rust integral type", *self).into(), - ) - } + _ => Err(anyhow!( + "can't convert {} bits into a Rust integral type", + *self + ))?, }, Span::call_site(), )) @@ -294,13 +293,10 @@ impl U32Ext for u32 { 9..=16 => 16, 17..=32 => 32, 33..=64 => 64, - _ => { - return Err(format!( - "can't convert {} bits into a Rust integral type width", - *self - ) - .into()) - } + _ => Err(anyhow!( + "can't convert {} bits into a Rust integral type width", + *self + ))?, }) } }