Merge #544

544: Better error handling r=burrbull a=robamu

Just a small attempt to make `svd2rust` print more useful information if something goes wrong in the TokenStream generation, or in general

- More debug output in case something goes wrong in the TokenStream generation
- Some info output in the main

I like how the `svd` XML parser handles errors by passing an Error Enum with a node ID up which can be used by one central error handler to print diagnostic information. I don't think it can be done like that here, so I simply print out some locally available information (peripheral name, register name etc.) as the error is propagated

Co-authored-by: Robin Mueller <robin.mueller.m@gmail.com>

Author: bors[bot]

CHANGELOG.md

@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   match the cluster layout.  Requires the `--const_generic` command line option.
 - Bumped `xtensa-lx` and add `xtensa_lx::interrupt::InterruptNumber` implementation.
 - Don't use a mask when the width of the mask is the same as the width of the parent register.
+- Improved error handling
 
 ## [v0.19.0] - 2021-05-26
 

src/generate/device.rs

@@ -1,12 +1,13 @@
 use crate::svd::Device;
 use proc_macro2::{Ident, Span, TokenStream};
 use quote::{quote, ToTokens};
+
 use std::fs::File;
 use std::io::Write;
 
 use crate::util::{self, Config, ToSanitizedUpperCase};
 use crate::Target;
-use anyhow::Result;
+use anyhow::{Context, Result};
 
 use crate::generate::{interrupt, peripheral};
 
@@ -191,12 +192,27 @@ pub fn render(d: &Device, config: &Config, device_x: &mut String) -> Result<Toke
             continue;
         }
 
-        out.extend(peripheral::render(
-            p,
-            &d.peripherals,
-            &d.default_register_properties,
-            config,
-        )?);
+        match peripheral::render(p, &d.peripherals, &d.default_register_properties, config) {
+            Ok(periph) => out.extend(periph),
+            Err(e) => {
+                let descrip = p.description.as_deref().unwrap_or("No description");
+                let group_name = p.group_name.as_deref().unwrap_or("No group name");
+                let mut context_string = format!(
+                    "Rendering error at peripheral\nName: {}\nDescription: {}\nGroup: {}",
+                    p.name, descrip, group_name
+                );
+                if p.derived_from.is_some() {
+                    context_string = format!(
+                        "{}\nDerived from: {}",
+                        context_string,
+                        p.derived_from.as_deref().unwrap()
+                    );
+                }
+                let mut e = Err(e);
+                e = e.with_context(|| context_string);
+                return e;
+            }
+        };
 
         if p.registers
             .as_ref()

src/generate/peripheral.rs

@@ -12,7 +12,8 @@ use quote::{quote, ToTokens};
 use syn::{parse_str, Token};
 
 use crate::util::{
-    self, Config, FullName, ToSanitizedSnakeCase, ToSanitizedUpperCase, BITS_PER_BYTE,
+    self, handle_cluster_error, handle_reg_error, Config, FullName, ToSanitizedSnakeCase,
+    ToSanitizedUpperCase, BITS_PER_BYTE,
 };
 use anyhow::{anyhow, bail, Context, Result};
 
@@ -194,14 +195,13 @@ pub fn render(
 
     // Push all register related information into the peripheral module
     for reg in registers {
-        mod_items.extend(register::render(
-            reg,
-            registers,
-            p,
-            all_peripherals,
-            &defaults,
-            config,
-        )?);
+        match register::render(reg, registers, p, all_peripherals, &defaults, config) {
+            Ok(rendered_reg) => mod_items.extend(rendered_reg),
+            Err(e) => {
+                let res: Result<TokenStream> = Err(e);
+                return handle_reg_error("Error rendering register", *reg, res);
+            }
+        };
     }
 
     let description =
@@ -454,7 +454,8 @@ fn register_or_cluster_block(
     let mut accessors = TokenStream::new();
     let mut have_accessors = false;
 
-    let ercs_expanded = expand(ercs, defs, name, config)?;
+    let ercs_expanded = expand(ercs, defs, name, config)
+        .with_context(|| "Could not expand register or cluster block")?;
 
     // Locate conflicting regions; we'll need to use unions to represent them.
     let mut regions = FieldRegions::default();
@@ -596,10 +597,30 @@ fn expand(
     let mut ercs_expanded = vec![];
 
     for erc in ercs {
-        ercs_expanded.extend(match &erc {
-            RegisterCluster::Register(register) => expand_register(register, defs, name, config)?,
-            RegisterCluster::Cluster(cluster) => expand_cluster(cluster, defs, name, config)?,
-        });
+        match &erc {
+            RegisterCluster::Register(register) => {
+                match expand_register(register, defs, name, config) {
+                    Ok(expanded_reg) => ercs_expanded.extend(expanded_reg),
+                    Err(e) => {
+                        let res = Err(e);
+                        return handle_reg_error("Error expanding register", register, res);
+                    }
+                }
+            }
+            RegisterCluster::Cluster(cluster) => {
+                match expand_cluster(cluster, defs, name, config) {
+                    Ok(expanded_cluster) => ercs_expanded.extend(expanded_cluster),
+                    Err(e) => {
+                        let res = Err(e);
+                        return handle_cluster_error(
+                            "Error expanding register cluster",
+                            cluster,
+                            res,
+                        );
+                    }
+                }
+            }
+        };
     }
 
     ercs_expanded.sort_by_key(|x| x.offset);
@@ -740,7 +761,8 @@ fn expand_register(
 
     match register {
         Register::Single(info) => register_expanded.push(RegisterBlockField {
-            field: convert_svd_register(register, name, config.ignore_groups)?,
+            field: convert_svd_register(register, name, config.ignore_groups)
+                .with_context(|| "syn error occured")?,
             description: info.description.clone().unwrap_or_default(),
             offset: info.address_offset,
             size: register_size,
@@ -815,14 +837,17 @@ fn cluster_block(
     // Generate definition for each of the registers.
     let registers = util::only_registers(&c.children);
     for reg in &registers {
-        mod_items.extend(register::render(
-            reg,
-            &registers,
-            p,
-            all_peripherals,
-            &defaults,
-            config,
-        )?);
+        match register::render(reg, &registers, p, all_peripherals, &defaults, config) {
+            Ok(rendered_reg) => mod_items.extend(rendered_reg),
+            Err(e) => {
+                let res: Result<TokenStream> = Err(e);
+                return handle_reg_error(
+                    "Error generating register definition for a register cluster",
+                    *reg,
+                    res,
+                );
+            }
+        };
     }
 
     // Generate the sub-cluster blocks.
@@ -848,7 +873,7 @@ fn expand_svd_register(
     register: &Register,
     name: Option<&str>,
     ignore_group: bool,
-) -> Result<Vec<syn::Field>, syn::Error> {
+) -> Result<Vec<syn::Field>> {
     let mut out = vec![];
 
     match register {
@@ -885,13 +910,14 @@ fn convert_svd_register(
     register: &Register,
     name: Option<&str>,
     ignore_group: bool,
-) -> Result<syn::Field, syn::Error> {
+) -> Result<syn::Field> {
     Ok(match register {
         Register::Single(info) => {
             let info_name = info.fullname(ignore_group);
             new_syn_field(
                 &info_name.to_sanitized_snake_case(),
-                name_to_wrapped_ty(&info_name, name)?,
+                name_to_wrapped_ty(&info_name, name)
+                    .with_context(|| format!("Error converting info name {}", info_name))?,
             )
         }
         Register::Array(info, array_info) => {
@@ -1038,7 +1064,16 @@ fn name_to_wrapped_ty_str(name: &str, ns: Option<&str>) -> String {
     }
 }
 
-fn name_to_wrapped_ty(name: &str, ns: Option<&str>) -> Result<syn::Type, syn::Error> {
+fn name_to_wrapped_ty(name: &str, ns: Option<&str>) -> Result<syn::Type> {
     let ident = name_to_wrapped_ty_str(name, ns);
-    Ok(syn::Type::Path(parse_str::<syn::TypePath>(&ident)?))
+    match parse_str::<syn::TypePath>(&ident) {
+        Ok(path) => Ok(syn::Type::Path(path)),
+        Err(e) => {
+            let mut res = Err(e.into());
+            res = res.with_context(|| {
+                format!("Determining syn::TypePath from ident \"{}\" failed", ident)
+            });
+            res
+        }
+    }
 }

src/main.rs

@@ -1,6 +1,6 @@
 #![recursion_limit = "128"]
 
-use log::error;
+use log::{error, info};
 use std::path::PathBuf;
 use svd_parser::svd;
 
@@ -164,10 +164,15 @@ fn run() -> Result<()> {
         svd::ValidateLevel::Weak
     };
 
-    let device = svd_parser::parse_with_config(xml, &parser_config)?;
+    info!("Parsing device from SVD file");
+    let device = svd_parser::parse_with_config(xml, &parser_config)
+        .with_context(|| "Error parsing SVD file".to_string())?;
 
     let mut device_x = String::new();
-    let items = generate::device::render(&device, &config, &mut device_x)?;
+    info!("Rendering device");
+    let items = generate::device::render(&device, &config, &mut device_x)
+        .with_context(|| "Error rendering device")?;
+
     let filename = if make_mod { "mod.rs" } else { "lib.rs" };
     let mut file = File::create(path.join(filename)).expect("Couldn't create output file");
 

src/util.rs

@@ -6,7 +6,7 @@ use proc_macro2::{Ident, Literal, Span, TokenStream};
 use quote::{quote, ToTokens};
 use std::path::PathBuf;
 
-use anyhow::{anyhow, bail, Result};
+use anyhow::{anyhow, bail, Context, Result};
 
 pub const BITS_PER_BYTE: u32 = 8;
 
@@ -370,6 +370,22 @@ pub fn build_rs() -> TokenStream {
     }
 }
 
+pub fn handle_reg_error<T>(msg: &str, reg: &Register, res: Result<T>) -> Result<T> {
+    let reg_name = &reg.name;
+    let descrip = reg.description.as_deref().unwrap_or("No description");
+    handle_erc_error(msg, reg_name, descrip, res)
+}
+
+pub fn handle_cluster_error<T>(msg: &str, cluster: &Cluster, res: Result<T>) -> Result<T> {
+    let cluster_name = &cluster.name;
+    let descrip = cluster.description.as_deref().unwrap_or("No description");
+    handle_erc_error(msg, cluster_name, descrip, res)
+}
+
+fn handle_erc_error<T>(msg: &str, name: &str, descrip: &str, res: Result<T>) -> Result<T> {
+    res.with_context(|| format!("{}\nName: {}\nDescription: {}", msg, name, descrip))
+}
+
 pub trait FullName {
     fn fullname(&self, ignore_group: bool) -> Cow<str>;
 }