Merge #715

715: cows, cows everywhere r=Emilgardis a=burrbull



Co-authored-by: Andrey Zgarbul <zgarbul.andrey@gmail.com>

Author: bors[bot]

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
 ## [Unreleased]
 
 - Bump MSRV to 1.65
+- Optimize case change/sanitize
 - Fix dangling implicit derives
 - Fix escaping <> and & characters in doc attributes
 

ci/svd2rust-regress/Cargo.toml

@@ -1,12 +1,12 @@
 [package]
-edition = "2018"
+edition = "2021"
 name = "svd2rust-regress"
 version = "0.1.0"
 authors = ["James Munns <james.munns@gmail.com>", "The svd2rust developers"]
 
 [dependencies]
-reqwest = { version = "0.10", features= ["blocking"] }
+clap = { version = "4.1", features = ["color", "derive"] }
+svd2rust = { path = "../../" }
+reqwest = { version = "0.11", features= ["blocking"] }
 rayon = "1.4"
-structopt = "0.2"
 error-chain = "0.12"
-inflections = "1.1"

ci/svd2rust-regress/src/main.rs

@@ -5,6 +5,7 @@ mod errors;
 mod svd_test;
 mod tests;
 
+use clap::Parser;
 use error_chain::ChainedError;
 use rayon::prelude::*;
 use std::fs::File;
@@ -13,92 +14,91 @@ use std::path::PathBuf;
 use std::process::{exit, Command};
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::time::Instant;
-use structopt::StructOpt;
 
-#[derive(StructOpt, Debug)]
-#[structopt(name = "svd2rust-regress")]
+#[derive(Parser, Debug)]
+#[command(name = "svd2rust-regress")]
 struct Opt {
     /// Run a long test (it's very long)
-    #[structopt(short = "l", long = "long-test")]
+    #[clap(short = 'l', long)]
     long_test: bool,
 
     /// Path to an `svd2rust` binary, relative or absolute.
     /// Defaults to `target/release/svd2rust[.exe]` of this repository
     /// (which must be already built)
-    #[structopt(short = "p", long = "svd2rust-path", parse(from_os_str))]
+    #[clap(short = 'p', long = "svd2rust-path")]
     bin_path: Option<PathBuf>,
 
     // TODO: Consider using the same strategy cargo uses for passing args to rustc via `--`
     /// Run svd2rust with `--atomics`
-    #[structopt(long = "atomics")]
+    #[clap(long)]
     atomics: bool,
 
     /// Filter by chip name, case sensitive, may be combined with other filters
-    #[structopt(short = "c", long = "chip", raw(validator = "validate_chips"))]
+    #[clap(short = 'c', long, value_parser = validate_chips)]
     chip: Vec<String>,
 
     /// Filter by manufacturer, case sensitive, may be combined with other filters
-    #[structopt(
-        short = "m",
+    #[clap(
+        short = 'm',
         long = "manufacturer",
-        raw(validator = "validate_manufacturer")
+        value_parser = validate_manufacturer,
     )]
     mfgr: Option<String>,
 
     /// Filter by architecture, case sensitive, may be combined with other filters
     /// Options are: "CortexM", "RiscV", "Msp430", "Mips" and "XtensaLX"
-    #[structopt(
-        short = "a",
+    #[clap(
+        short = 'a',
         long = "architecture",
-        raw(validator = "validate_architecture")
+        value_parser = validate_architecture,
     )]
     arch: Option<String>,
 
     /// Include tests expected to fail (will cause a non-zero return code)
-    #[structopt(short = "b", long = "bad-tests")]
+    #[clap(short = 'b', long)]
     bad_tests: bool,
 
     /// Enable formatting with `rustfmt`
-    #[structopt(short = "f", long = "format")]
+    #[clap(short = 'f', long)]
     format: bool,
 
     /// Print all available test using the specified filters
-    #[structopt(long = "list")]
+    #[clap(long)]
     list: bool,
 
     /// Path to an `rustfmt` binary, relative or absolute.
     /// Defaults to `$(rustup which rustfmt)`
-    #[structopt(long = "rustfmt_bin_path", parse(from_os_str))]
+    #[clap(long)]
     rustfmt_bin_path: Option<PathBuf>,
 
     /// Specify what rustup toolchain to use when compiling chip(s)
-    #[structopt(long = "toolchain", env = "RUSTUP_TOOLCHAIN")]
+    #[clap(long = "toolchain")] // , env = "RUSTUP_TOOLCHAIN"
     rustup_toolchain: Option<String>,
 
     /// Use verbose output
-    #[structopt(long = "verbose", short = "v", parse(from_occurrences))]
+    #[clap(long, short = 'v', action = clap::ArgAction::Count)]
     verbose: u8,
     // TODO: Specify smaller subset of tests? Maybe with tags?
     // TODO: Compile svd2rust?
 }
 
-fn validate_chips(s: String) -> Result<(), String> {
+fn validate_chips(s: &str) -> 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: &str) -> Result<(), String> {
     if tests::TESTS.iter().any(|t| format!("{:?}", t.arch) == s) {
         Ok(())
     } else {
         Err(format!("Architecture `{s}` is not a valid value"))
     }
 }
 
-fn validate_manufacturer(s: String) -> Result<(), String> {
+fn validate_manufacturer(s: &str) -> Result<(), String> {
     if tests::TESTS.iter().any(|t| format!("{:?}", t.mfgr) == s) {
         Ok(())
     } else {
@@ -140,7 +140,7 @@ fn read_file(path: &PathBuf, buf: &mut String) {
 }
 
 fn main() {
-    let opt = Opt::from_args();
+    let opt = Opt::parse();
 
     // Validate all test pre-conditions
     validate_tests(tests::TESTS);

ci/svd2rust-regress/src/svd_test.rs

@@ -127,15 +127,16 @@ pub fn test(
         .open(svd_toml)
         .chain_err(|| "Failed to open Cargo.toml for appending")?;
 
-    use crate::tests::Architecture::*;
+    use crate::tests::Target;
     let crates = CRATES_ALL
         .iter()
         .chain(match &t.arch {
-            CortexM => CRATES_CORTEX_M.iter(),
-            RiscV => CRATES_RISCV.iter(),
-            Mips => CRATES_MIPS.iter(),
-            Msp430 => CRATES_MSP430.iter(),
-            XtensaLX => CRATES_XTENSALX.iter(),
+            Target::CortexM => CRATES_CORTEX_M.iter(),
+            Target::RISCV => CRATES_RISCV.iter(),
+            Target::Mips => CRATES_MIPS.iter(),
+            Target::Msp430 => CRATES_MSP430.iter(),
+            Target::XtensaLX => CRATES_XTENSALX.iter(),
+            Target::None => unreachable!(),
         })
         .chain(if atomics {
             CRATES_ATOMICS.iter()
@@ -145,7 +146,7 @@ pub fn test(
         .chain(PROFILE_ALL.iter())
         .chain(FEATURES_ALL.iter())
         .chain(match &t.arch {
-            XtensaLX => FEATURES_XTENSALX.iter(),
+            Target::XtensaLX => FEATURES_XTENSALX.iter(),
             _ => [].iter(),
         });
 
@@ -170,11 +171,12 @@ pub fn test(
     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 {
-        CortexM => "cortex-m",
-        Msp430 => "msp430",
-        Mips => "mips",
-        RiscV => "riscv",
-        XtensaLX => "xtensa-lx",
+        Target::CortexM => "cortex-m",
+        Target::Msp430 => "msp430",
+        Target::Mips => "mips",
+        Target::RISCV => "riscv",
+        Target::XtensaLX => "xtensa-lx",
+        Target::None => unreachable!(),
     };
     let mut svd2rust_bin = Command::new(bin_path);
     if atomics {
@@ -190,15 +192,18 @@ pub fn test(
     output.capture_outputs(
         true,
         "svd2rust",
-        Some(&lib_rs_file)
-            .filter(|_| (t.arch != CortexM) && (t.arch != Msp430) && (t.arch != XtensaLX)),
+        Some(&lib_rs_file).filter(|_| {
+            (t.arch != Target::CortexM)
+                && (t.arch != Target::Msp430)
+                && (t.arch != Target::XtensaLX)
+        }),
         Some(&svd2rust_err_file),
         &[],
     )?;
     process_stderr_paths.push(svd2rust_err_file);
 
     match t.arch {
-        CortexM | Mips | Msp430 | XtensaLX => {
+        Target::CortexM | Target::Mips | Target::Msp430 | Target::XtensaLX => {
             // 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")?

ci/svd2rust-regress/src/tests.rs

@@ -1,16 +1,5 @@
-use inflections::Inflect;
-use std::borrow::Cow;
-
-#[derive(Debug, PartialEq)]
-pub enum Architecture {
-    // TODO: Coming soon!
-    // Avr,
-    CortexM,
-    Mips,
-    Msp430,
-    RiscV,
-    XtensaLX,
-}
+pub use svd2rust::util::Target;
+use svd2rust::util::ToSanitizedCase;
 
 #[derive(Debug)]
 pub enum Manufacturer {
@@ -41,7 +30,7 @@ pub enum RunWhen {
 }
 
 pub struct TestCase {
-    pub arch: Architecture,
+    pub arch: Target,
     pub mfgr: Manufacturer,
     pub chip: &'static str,
     svd_url: Option<&'static str>,
@@ -75,59 +64,9 @@ impl TestCase {
     }
 }
 
-use self::Architecture::*;
 use self::Manufacturer::*;
 use self::RunWhen::*;
-
-/// List of chars that some vendors use in their peripheral/field names but
-/// that are not valid in Rust ident
-const BLACKLIST_CHARS: &[char] = &['(', ')', '[', ']'];
-
-/// Lovingly stolen from `svd2rust`
-pub trait ToSanitizedCase {
-    fn to_sanitized_not_keyword_snake_case(&self) -> Cow<str>;
-    fn to_sanitized_snake_case(&self) -> Cow<str> {
-        let s = self.to_sanitized_not_keyword_snake_case();
-        sanitize_keyword(s)
-    }
-}
-
-impl ToSanitizedCase for str {
-    fn to_sanitized_not_keyword_snake_case(&self) -> Cow<str> {
-        const INTERNALS: [&str; 4] = ["set_bit", "clear_bit", "bit", "bits"];
-
-        let s = self.replace(BLACKLIST_CHARS, "");
-        match s.chars().next().unwrap_or('\0') {
-            '0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' => {
-                format!("_{}", s.to_snake_case()).into()
-            }
-            _ => {
-                let s = Cow::from(s.to_snake_case());
-                if INTERNALS.contains(&s.as_ref()) {
-                    s + "_"
-                } else {
-                    s
-                }
-            }
-        }
-    }
-}
-
-pub fn sanitize_keyword(sc: Cow<str>) -> Cow<str> {
-    const KEYWORDS: [&str; 55] = [
-        "abstract", "alignof", "as", "async", "await", "become", "box", "break", "const",
-        "continue", "crate", "do", "dyn", "else", "enum", "extern", "false", "final", "fn", "for",
-        "if", "impl", "in", "let", "loop", "macro", "match", "mod", "move", "mut", "offsetof",
-        "override", "priv", "proc", "pub", "pure", "ref", "return", "self", "sizeof", "static",
-        "struct", "super", "trait", "true", "try", "type", "typeof", "unsafe", "unsized", "use",
-        "virtual", "where", "while", "yield",
-    ];
-    if KEYWORDS.contains(&sc.as_ref()) {
-        sc + "_"
-    } else {
-        sc
-    }
-}
+use self::Target::{CortexM, Mips, Msp430, XtensaLX, RISCV};
 
 // NOTE: All chip names must be unique!
 pub const TESTS: &[&TestCase] = &[
@@ -4175,7 +4114,7 @@ pub const TESTS: &[&TestCase] = &[
         run_when: Always,
     },
     &TestCase {
-        arch: RiscV,
+        arch: RISCV,
         mfgr: SiFive,
         chip: "E310x",
         svd_url: Some("https://raw.githubusercontent.com/riscv-rust/e310x/master/e310x.svd"),
@@ -4231,7 +4170,7 @@ pub const TESTS: &[&TestCase] = &[
         run_when: Always,
     },
     &TestCase {
-        arch: RiscV,
+        arch: RISCV,
         mfgr: Espressif,
         chip: "esp32c3",
         svd_url: Some(