Merge #401

401: MSP430 Improvements r=therealprof a=cr1901

This PR brings MSP430 PAC generation up to parity with Cortex M PAC generation. Among other things, a `device.x` file is now generated for each MSP430 PAC, and the `interrupt!` macro has been replaced with an attribute macro from the not-yet-published `msp430-rt v0.2.0`. 

The remaining changes are docs and cleanups of the generated code; a number of features have become stable. Only `abi_msp430_interrupt` is not stable yet, and I have a workaround for this that I've not written yet.

Switching to an `interrupt` attribute macro is a breaking change, so I incremented the "major" version number.

Co-authored-by: William D. Jones <thor0505@comcast.net>

Author: bors[bot]

CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Changed
+
+- [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`.
+
 ## [v0.17.0] - 2019-12-31
 
 ### Fixed

ci/svd2rust-regress/src/errors.rs

@@ -1,5 +1,5 @@
 use std::path::PathBuf;
-error_chain!{
+error_chain! {
     errors {
         ProcessFailed(command: String, stderr: Option<PathBuf>, stdout: Option<PathBuf>, previous_processes_stderr: Vec<PathBuf>) {
             description("Process Failed")

ci/svd2rust-regress/src/main.rs

@@ -27,7 +27,7 @@ struct Opt {
     /// (which must be already built)
     #[structopt(short = "p", long = "svd2rust-path", parse(from_os_str))]
     bin_path: Option<PathBuf>,
-    
+
     // TODO: Consider using the same strategy cargo uses for passing args to rustc via `--`
     /// Run svd2rust with `--nightly`
     #[structopt(long = "nightly")]
@@ -38,12 +38,20 @@ struct Opt {
     chip: Vec<String>,
 
     /// Filter by manufacturer, case sensitive, may be combined with other filters
-    #[structopt(short = "m", long = "manufacturer", raw(validator = "validate_manufacturer"))]
+    #[structopt(
+        short = "m",
+        long = "manufacturer",
+        raw(validator = "validate_manufacturer")
+    )]
     mfgr: Option<String>,
 
     /// Filter by architecture, case sensitive, may be combined with other filters
     /// Options are: "CortexM", "RiscV", and "Msp430"
-    #[structopt(short = "a", long = "architecture", raw(validator = "validate_architecture"))]
+    #[structopt(
+        short = "a",
+        long = "architecture",
+        raw(validator = "validate_architecture")
+    )]
     arch: Option<String>,
 
     /// Include tests expected to fail (will cause a non-zero return code)
@@ -74,23 +82,23 @@ struct Opt {
     // TODO: Compile svd2rust?
 }
 
-fn validate_chips(s: String) -> Result<(), String>{
+fn validate_chips(s: String) -> Result<(), String> {
     if tests::TESTS.iter().any(|t| t.chip == s) {
         Ok(())
     } else {
         Err(format!("Chip `{}` is not a valid value", s))
     }
 }
 
-fn validate_architecture(s: String) -> Result<(), String>{
+fn validate_architecture(s: String) -> Result<(), String> {
     if tests::TESTS.iter().any(|t| format!("{:?}", t.arch) == s) {
         Ok(())
     } else {
         Err(format!("Architecture `{}` is not a valid value", s))
     }
 }
 
-fn validate_manufacturer(s: String) -> Result<(), String>{
+fn validate_manufacturer(s: String) -> Result<(), String> {
     if tests::TESTS.iter().any(|t| format!("{:?}", t.mfgr) == s) {
         Ok(())
     } else {
@@ -144,7 +152,8 @@ fn main() {
         default_svd2rust_iter.iter().chain(["svd2rust.exe"].iter())
     } else {
         default_svd2rust_iter.iter().chain(["svd2rust"].iter())
-    }.collect();
+    }
+    .collect();
 
     let bin_path = match opt.bin_path {
         Some(ref bp) => bp,
@@ -170,11 +179,7 @@ fn main() {
         (&Some(ref path), true) => Some(path),
         (&None, true) => {
             // FIXME: Use Option::filter instead when stable, rust-lang/rust#45860
-            if default_rustfmt
-                .iter()
-                .find(|p| p.is_file())
-                .is_none()
-            {
+            if default_rustfmt.iter().find(|p| p.is_file()).is_none() {
                 panic!("No rustfmt found");
             }
             default_rustfmt.as_ref()
@@ -260,8 +265,13 @@ fn main() {
                 any_fails.store(true, Ordering::Release);
                 let additional_info = if opt.verbose > 0 {
                     match *e.kind() {
-                        errors::ErrorKind::ProcessFailed(_, _, Some(ref stderr), ref previous_processes_stderr) => {
-                            let mut buf = String::new(); 
+                        errors::ErrorKind::ProcessFailed(
+                            _,
+                            _,
+                            Some(ref stderr),
+                            ref previous_processes_stderr,
+                        ) => {
+                            let mut buf = String::new();
                             if opt.verbose > 1 {
                                 for stderr in previous_processes_stderr {
                                     read_file(&stderr, &mut buf);

ci/svd2rust-regress/src/svd_test.rs

@@ -1,18 +1,19 @@
 use crate::errors::*;
+use crate::tests::TestCase;
 use reqwest;
 use std::fs::{self, File, OpenOptions};
 use std::io::prelude::*;
 use std::path::PathBuf;
 use std::process::{Command, Output};
-use crate::tests::TestCase;
 
 static CRATES_ALL: &[&str] = &["bare-metal = \"0.2.0\"", "vcell = \"0.1.0\""];
 static CRATES_MSP430: &[&str] = &["msp430 = \"0.1.0\""];
 static CRATES_CORTEX_M: &[&str] = &["cortex-m = \"0.5.0\"", "cortex-m-rt = \"0.5.0\""];
 static CRATES_RISCV: &[&str] = &["riscv = \"0.4.0\"", "riscv-rt = \"0.4.0\""];
 static PROFILE_ALL: &[&str] = &["[profile.dev]", "incremental = false"];
 static FEATURES_ALL: &[&str] = &["[features]"];
-static FEATURES_CORTEX_M: &[&str] = &["const-fn = [\"bare-metal/const-fn\", \"cortex-m/const-fn\"]"];
+static FEATURES_CORTEX_M: &[&str] =
+    &["const-fn = [\"bare-metal/const-fn\", \"cortex-m/const-fn\"]"];
 static FEATURES_EMPTY: &[&str] = &[];
 
 fn path_helper(input: &[&str]) -> PathBuf {
@@ -66,20 +67,26 @@ impl CommandHelper for Output {
         };
 
         if cant_fail && !self.status.success() {
-            return Err(
-                ErrorKind::ProcessFailed(name.into(),
-                    stdout.cloned(),
-                    stderr.cloned(),
-                    previous_processes_stderr.to_vec(),
-                ).into()
-            );
+            return Err(ErrorKind::ProcessFailed(
+                name.into(),
+                stdout.cloned(),
+                stderr.cloned(),
+                previous_processes_stderr.to_vec(),
+            )
+            .into());
         }
 
         Ok(())
     }
 }
 
-pub fn test(t: &TestCase, bin_path: &PathBuf, rustfmt_bin_path: Option<&PathBuf>, nightly: bool, verbosity: u8) -> Result<Option<Vec<PathBuf>>> {
+pub fn test(
+    t: &TestCase,
+    bin_path: &PathBuf,
+    rustfmt_bin_path: Option<&PathBuf>,
+    nightly: bool,
+    verbosity: u8,
+) -> Result<Option<Vec<PathBuf>>> {
     let user = match std::env::var("USER") {
         Ok(val) => val,
         Err(_) => "rusttester".into(),
@@ -90,7 +97,7 @@ pub fn test(t: &TestCase, bin_path: &PathBuf, rustfmt_bin_path: Option<&PathBuf>
     if let Err(err) = fs::remove_dir_all(&chip_dir) {
         match err.kind() {
             std::io::ErrorKind::NotFound => (),
-            _ => Err(err).chain_err(|| "While removing chip directory")?
+            _ => Err(err).chain_err(|| "While removing chip directory")?,
         }
     }
 
@@ -151,7 +158,7 @@ pub fn test(t: &TestCase, bin_path: &PathBuf, rustfmt_bin_path: Option<&PathBuf>
     file_helper(&svd, &svd_file)?;
 
     // Generate the lib.rs from the SVD file using the specified `svd2rust` binary
-    // If the architecture is cortex-m we move the generated lib.rs file to src/
+    // If the architecture is cortex-m or msp430 we move the generated lib.rs file to src/
     let lib_rs_file = path_helper_base(&chip_dir, &["src", "lib.rs"]);
     let svd2rust_err_file = path_helper_base(&chip_dir, &["svd2rust.err.log"]);
     let target = match t.arch {
@@ -173,21 +180,25 @@ pub fn test(t: &TestCase, bin_path: &PathBuf, rustfmt_bin_path: Option<&PathBuf>
     output.capture_outputs(
         true,
         "svd2rust",
-        if t.arch != CortexM { Some(&lib_rs_file) } else { None }, // use Option.filter
+        Some(&lib_rs_file).filter(|_| (t.arch != CortexM) && (t.arch != Msp430)),
         Some(&svd2rust_err_file),
         &[],
     )?;
     process_stderr_paths.push(svd2rust_err_file);
 
-    if let CortexM = t.arch {
-        // TODO: Give error the path to stderr
-        fs::rename(path_helper_base(&chip_dir, &["lib.rs"]), &lib_rs_file).chain_err(|| "While moving lib.rs file")?
+    match t.arch {
+        CortexM | Msp430 => {
+            // TODO: Give error the path to stderr
+            fs::rename(path_helper_base(&chip_dir, &["lib.rs"]), &lib_rs_file)
+                .chain_err(|| "While moving lib.rs file")?
+        }
+        _ => {}
     }
 
     let rustfmt_err_file = path_helper_base(&chip_dir, &["rustfmt.err.log"]);
     if let Some(rustfmt_bin_path) = rustfmt_bin_path {
         // Run `cargo fmt`, capturing stderr to a log file
-        
+
         let output = Command::new(rustfmt_bin_path)
             .arg(lib_rs_file)
             .output()
@@ -200,7 +211,6 @@ pub fn test(t: &TestCase, bin_path: &PathBuf, rustfmt_bin_path: Option<&PathBuf>
             &process_stderr_paths,
         )?;
         process_stderr_paths.push(rustfmt_err_file);
-
     }
     // Run `cargo check`, capturing stderr to a log file
     let cargo_check_err_file = path_helper_base(&chip_dir, &["cargo-check.err.log"]);
@@ -215,7 +225,11 @@ pub fn test(t: &TestCase, bin_path: &PathBuf, rustfmt_bin_path: Option<&PathBuf>
         None,
         Some(&cargo_check_err_file),
         &process_stderr_paths,
-        )?;
+    )?;
     process_stderr_paths.push(cargo_check_err_file);
-    Ok(if verbosity > 1 {Some(process_stderr_paths)} else {None})
+    Ok(if verbosity > 1 {
+        Some(process_stderr_paths)
+    } else {
+        None
+    })
 }

ci/svd2rust-regress/src/tests.rs

@@ -709,7 +709,9 @@ pub const TESTS: &[&TestCase] = &[
         arch: CortexM,
         mfgr: Atmel,
         chip: "ATSAMD21G18A",
-        svd_url: Some("https://raw.githubusercontent.com/wez/atsamd21-rs/master/svd/ATSAMD21G18A.svd"),
+        svd_url: Some(
+            "https://raw.githubusercontent.com/wez/atsamd21-rs/master/svd/ATSAMD21G18A.svd",
+        ),
         should_pass: true,
         run_when: NotShort,
     },
@@ -4220,7 +4222,7 @@ pub const TESTS: &[&TestCase] = &[
         arch: Msp430,
         mfgr: TexasInstruments,
         chip: "msp430g2553",
-        svd_url: Some("https://github.com/pftbest/msp430g2553/raw/v0.1.0/msp430g2553.svd"),
+        svd_url: Some("https://github.com/pftbest/msp430g2553/raw/v0.1.3-svd/msp430g2553.svd"),
         should_pass: true,
         run_when: Always,
     },

src/generate/device.rs

@@ -35,14 +35,6 @@ pub fn render(
         });
     }
 
-    if target != Target::None && target != Target::CortexM && target != Target::RISCV {
-        out.extend(quote! {
-            #![cfg_attr(feature = "rt", feature(global_asm))]
-            #![cfg_attr(feature = "rt", feature(use_extern_macros))]
-            #![cfg_attr(feature = "rt", feature(used))]
-        });
-    }
-
     out.extend(quote! {
         #![doc = #doc]
         // Deny a subset of warnings
@@ -84,8 +76,6 @@ pub fn render(
                 extern crate msp430;
                 #[cfg(feature = "rt")]
                 extern crate msp430_rt;
-                #[cfg(feature = "rt")]
-                pub use msp430_rt::default_handler;
             });
         }
         Target::RISCV => {
@@ -158,6 +148,17 @@ pub fn render(
         }
     }
 
+    if target == Target::Msp430 {
+        out.extend(quote! {
+            // XXX: Are there any core peripherals, really? Requires bump of msp430 crate.
+            // pub use msp430::peripheral::Peripherals as CorePeripherals;
+            #[cfg(feature = "rt")]
+            pub use msp430_rt::interrupt;
+            #[cfg(feature = "rt")]
+            pub use self::Interrupt as interrupt;
+        });
+    }
+
     let generic_file = std::str::from_utf8(include_bytes!("generic.rs")).unwrap();
     if generic_mod {
         writeln!(File::create("generic.rs").unwrap(), "{}", generic_file).unwrap();

src/generate/interrupt.rs

@@ -97,29 +97,11 @@ pub fn render(
             });
         }
         Target::Msp430 => {
-            let aliases = names
-                .iter()
-                .map(|n| {
-                    format!(
-                        "
-.weak {0}
-{0} = DH_TRAMPOLINE",
-                        n
-                    )
-                })
-                .collect::<Vec<_>>()
-                .concat();
-
-            mod_items.push(quote! {
-                #[cfg(feature = "rt")]
-                global_asm!("
-                DH_TRAMPOLINE:
-                    br #DEFAULT_HANDLER
-                ");
-
-                #[cfg(feature = "rt")]
-                global_asm!(#aliases);
+            for name in &names {
+                writeln!(device_x, "PROVIDE({} = DefaultHandler);", name).unwrap();
+            }
 
+            root.extend(quote! {
                 #[cfg(feature = "rt")]
                 extern "msp430-interrupt" {
                     #(fn #names();)*
@@ -136,7 +118,7 @@ pub fn render(
                 #[link_section = ".vector_table.interrupts"]
                 #[no_mangle]
                 #[used]
-                pub static INTERRUPTS:
+                pub static __INTERRUPTS:
                     [Vector; #n] = [
                         #(#elements,)*
                     ];
@@ -169,7 +151,7 @@ pub fn render(
         }
     };
 
-    if target == Target::CortexM {
+    if target == Target::CortexM || target == Target::Msp430 {
         root.extend(interrupt_enum);
     } else {
         mod_items.push(quote! {
@@ -196,7 +178,7 @@ pub fn render(
             _ => "C",
         };
 
-        if target != Target::CortexM {
+        if target != Target::CortexM && target != Target::Msp430 {
             mod_items.push(quote! {
                 #[cfg(feature = "rt")]
                 #[macro_export]
@@ -283,7 +265,7 @@ pub fn render(
         }
     }
 
-    if !interrupts.is_empty() && target != Target::CortexM {
+    if !interrupts.is_empty() && target != Target::CortexM && target != Target::Msp430 {
         root.extend(quote! {
             #[doc(hidden)]
             pub mod interrupt {