From 5e11aa6473d2ee5982ebcfb12fa7eb82d8ef9635 Mon Sep 17 00:00:00 2001 From: Tobias Tom Hodapp Date: Tue, 19 Nov 2024 13:48:51 +0100 Subject: [PATCH 1/3] Improved cli argument handling --- Cargo.toml | 2 +- src/config.rs | 226 +++++++++++++++++++++++++++----------------------- src/main.rs | 30 ++++--- 3 files changed, 142 insertions(+), 116 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 03bc5e9..39d2be6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,7 @@ members = [ ariadne = "0.4.1" # for nice errors num = "0.4" # itertools = "0.13.0" -clap = "3.2" +clap = { version = "4.5.21", features = ["derive"] } arrayvec = "0.7.6" # Tree sitter diff --git a/src/config.rs b/src/config.rs index 9aa6803..145029a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,21 +1,27 @@ -use std::{cell::UnsafeCell, collections::HashSet, env, ffi::OsStr, path::PathBuf}; - -use clap::{Arg, Command}; +use clap::{Arg, Command, ValueEnum}; +use std::{ + collections::HashSet, + env, + ffi::{OsStr, OsString}, + path::PathBuf, + sync::LazyLock, +}; -/// Describes at what point in the compilation process we should exit early. -/// -/// This is mainly to aid in debugging, where incorrect results from flattening/typechecking may lead to errors, +/// Describes at what point in the compilation process we should exit early. +/// +/// This is mainly to aid in debugging, where incorrect results from flattening/typechecking may lead to errors, /// which we still wish to see in say the LSP -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, ValueEnum)] pub enum EarlyExitUpTo { Initialize, Flatten, AbstractTypecheck, Lint, Instantiate, - CodeGen + CodeGen, } +#[derive(Debug, PartialEq, Eq)] pub struct ConfigStruct { pub use_lsp: bool, pub lsp_debug_mode: bool, @@ -26,73 +32,71 @@ pub struct ConfigStruct { pub debug_whitelist: Option>, pub codegen_module_and_dependencies_one_file: Option, pub early_exit: EarlyExitUpTo, - /// For terminal environments where color is not supported - pub use_color: bool + pub use_color: bool, + pub files: Vec, } -pub fn config() -> &'static ConfigStruct { - unsafe { &*CONFIG.cf.get() } -} - -pub fn parse_args() -> Vec { - let matches = Command::new("SUS Compiler") +fn command_builder() -> Command { + Command::new("SUS Compiler") .version(env!("CARGO_PKG_VERSION")) .author(env!("CARGO_PKG_AUTHORS")) .about("The compiler for the SUS Hardware Design Language. This compiler takes in .sus files, and produces equivalent SystemVerilog files") .arg(Arg::new("socket") .long("socket") - .takes_value(true) .default_value("25000") .help("Set the LSP TCP socket port") - .validator(|socket_int : &str| { + .value_parser(|socket_int : &str| { match u16::from_str_radix(socket_int, 10) { - Ok(_) => Ok(()), + Ok(port) => Ok(port), Err(_) => Err("Must be a valid port 0-65535") } }) .requires("lsp")) .arg(Arg::new("lsp") .long("lsp") - .help("Enable LSP mode")) + .help("Enable LSP mode") + .action(clap::ArgAction::SetTrue)) .arg(Arg::new("lsp-debug") .long("lsp-debug") .hide(true) .help("Enable LSP debug mode") - .requires("lsp")) + .requires("lsp") + .action(clap::ArgAction::SetTrue)) .arg(Arg::new("codegen") .long("codegen") - .help("Enable code generation for all modules. This creates a file named [ModuleName].sv per module.")) + .help("Enable code generation for all modules. This creates a file named [ModuleName].sv per module.") + .action(clap::ArgAction::SetTrue)) .arg(Arg::new("debug") .long("debug") .hide(true) - .help("Print debug information about the module contents")) + .help("Print debug information about the module contents") + .action(clap::ArgAction::SetTrue)) .arg(Arg::new("debug-latency") .long("debug-latency") .hide(true) - .help("Print latency graph for debugging")) + .help("Print latency graph for debugging") + .action(clap::ArgAction::SetTrue)) .arg(Arg::new("debug-whitelist") .long("debug-whitelist") .hide(true) .help("Sets the modules that should be shown by --debug. When not provided all modules are whitelisted") - .takes_value(true) - .multiple_values(true)) + .action(clap::ArgAction::Append)) .arg(Arg::new("standalone") .long("standalone") - .takes_value(true) - .help("Generate standalone code with all dependencies in one file of the module specified. ")) + .help("Generate standalone code with all dependencies in one file of the module specified.")) .arg(Arg::new("upto") .long("upto") .help("Describes at what point in the compilation process we should exit early. This is mainly to aid in debugging, where incorrect results from flattening/typechecking may lead to errors, which we still wish to see in say the LSP") - .takes_value(true) - .possible_values(&["initialize", "flatten", "typecheck", "lint", "instantiate", "codegen"]) - .default_value("codegen")) + .value_parser(clap::builder::EnumValueParser::::new()) + .default_value("code-gen")) .arg(Arg::new("nocolor") .long("nocolor") - .help("Disables color printing in the errors of the sus_compiler output")) + .help("Disables color printing in the errors of the sus_compiler output") + .action(clap::ArgAction::SetTrue)) .arg(Arg::new("files") - .multiple_values(true) + .action(clap::ArgAction::Append) .help(".sus Files") - .validator(|file_path_str : &str| { + .value_parser(|file_path_str : &str| { let file_path = PathBuf::from(file_path_str); if !file_path.exists() { Err("File does not exist") @@ -101,86 +105,102 @@ pub fn parse_args() -> Vec { } else if file_path.extension() != Some(OsStr::new("sus")) { Err("Source files must end in .sus") } else { - Ok(()) + Ok(file_path) } })) - .get_matches(); - - let config = unsafe { &mut *CONFIG.cf.get() }; - - if let Some(socket) = matches.value_of("socket") { - config.lsp_port = u16::from_str_radix(socket, 10).unwrap(); - } +} - config.use_lsp = matches.is_present("lsp"); - config.lsp_debug_mode = matches.is_present("lsp-debug"); - config.codegen = matches.is_present("codegen"); - config.debug_print_module_contents = matches.is_present("debug"); - config.debug_print_latency_graph = matches.is_present("debug-latency"); - if let Some(s) = matches.get_many("debug-whitelist") { - config.debug_whitelist = Some(s.map(|s: &String| s.clone()).collect()) - } - if matches.is_present("nocolor") { - config.use_color = false; - } - if config.use_lsp { - // Disable color because LSP output doesn't support it - config.use_color = false; - } - config.early_exit = match matches.value_of("upto").unwrap() { - "initialize" => EarlyExitUpTo::Initialize, - "flatten" => EarlyExitUpTo::Flatten, - "typecheck" => EarlyExitUpTo::AbstractTypecheck, - "lint" => EarlyExitUpTo::Lint, - "instantiate" => EarlyExitUpTo::Instantiate, - "codegen" => EarlyExitUpTo::CodeGen, - _ => unreachable!() +fn parse_args(itr: I) -> Result +where + I: IntoIterator, + T: Into + Clone, +{ + let matches = command_builder().try_get_matches_from(itr)?; + let lsp_port = *matches.get_one("socket").unwrap(); + let use_lsp = matches.get_flag("lsp"); + let lsp_debug_mode = matches.get_flag("lsp-debug"); + + let codegen = matches.get_flag("codegen") || matches.get_many::("files").is_none(); + let debug_print_module_contents = matches.get_flag("debug"); + let debug_print_latency_graph = matches.get_flag("debug-latency"); + let debug_whitelist = matches + .get_many("debug-whitelist") + .map(|s| s.cloned().collect()); + let use_color = !matches.get_flag("nocolor") && !use_lsp; + let early_exit = *matches.get_one("upto").unwrap(); + let codegen_module_and_dependencies_one_file = matches.get_one("standalone").cloned(); + let file_paths: Vec = match matches.get_many("files") { + Some(files) => files.cloned().collect(), + None => std::fs::read_dir(".") + .unwrap() + .map(|file| file.unwrap().path()) + .filter(|file_path| { + file_path.is_file() && file_path.extension() == Some("sus".as_ref()) + }) + .collect(), }; + Ok(ConfigStruct { + use_lsp, + lsp_debug_mode, + lsp_port, + codegen, + debug_print_module_contents, + debug_print_latency_graph, + debug_whitelist, + codegen_module_and_dependencies_one_file, + early_exit, + use_color, + files: file_paths, + }) +} +pub fn config() -> &'static ConfigStruct { + static CONFIG: LazyLock = LazyLock::new(|| { + let args: Vec = std::env::args_os().collect(); + parse_args(args.as_slice()) + .map_err(|err| err.exit()) + .unwrap() + }); + &CONFIG +} - if let Some(standalone) = matches.value_of("standalone") { - config.codegen_module_and_dependencies_one_file = Some(standalone.to_string()); - } +#[cfg(test)] +mod tests { + use super::parse_args; - let mut file_paths: Vec = Vec::new(); - if let Some(files) = matches.values_of("files") { - for file in files { - file_paths.push(PathBuf::from(file)); - } + #[test] + fn test_socket_invalid_port() { + let config = parse_args(&["", "--lsp", "--socket", "1234567890"]); + assert!(config.is_err()); + let err = config.unwrap_err(); + assert_eq!(err.kind(), clap::error::ErrorKind::ValueValidation); } - // For debugging, if no files are provided - if file_paths.is_empty() { - for file in std::fs::read_dir(".").unwrap() { - let file_path = file.unwrap().path(); - if file_path.is_file() && file_path.extension() == Some(OsStr::new("sus")) { - file_paths.push(file_path); - } - } - config.codegen = true; + #[test] + fn test_socket_require_lsp() { + let config = parse_args(&["", "--socket", "1500"]); + assert!(config.is_err()); + let err = config.unwrap_err(); + assert_eq!(err.kind(), clap::error::ErrorKind::MissingRequiredArgument); } - file_paths -} + #[test] + fn test_lsp_debug_require_lsp() { + let config = parse_args(&["", "--lsp-debug"]); + assert!(config.is_err()); + let err = config.unwrap_err(); + assert_eq!(err.kind(), clap::error::ErrorKind::MissingRequiredArgument); + } + #[test] + fn test_lsp_no_color() { + let config = parse_args(&["", "--lsp"]).unwrap(); + assert_eq!(config.use_color, false) + } -struct ConfigStructWrapper { - cf: UnsafeCell, + #[test] + fn test_automatic_codegen() { + let config = parse_args(&[""]).unwrap(); + assert_eq!(config.codegen, true) + } } - -unsafe impl Sync for ConfigStructWrapper {} - -static CONFIG: ConfigStructWrapper = ConfigStructWrapper { - cf: UnsafeCell::new(ConfigStruct { - use_lsp: false, - lsp_port: 25000, - lsp_debug_mode: false, - debug_print_module_contents: false, - codegen: false, - debug_print_latency_graph: false, - codegen_module_and_dependencies_one_file: None, - early_exit: EarlyExitUpTo::CodeGen, - use_color: true, - debug_whitelist: None, - }), -}; diff --git a/src/main.rs b/src/main.rs index 828b069..558fa8e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,17 +22,17 @@ mod linker; mod compiler_top; -use std::path::PathBuf; -use std::{error::Error, fs}; use std::fs::File; use std::io::Write; use std::ops::Deref; +use std::path::PathBuf; use std::rc::Rc; +use std::{error::Error, fs}; use prelude::*; use codegen_fallback::gen_verilog_code; -use config::{config, parse_args, EarlyExitUpTo}; +use config::{config, EarlyExitUpTo}; use dev_aid::ariadne_interface::*; use flattening::Module; use instantiation::InstantiatedModule; @@ -48,16 +48,20 @@ fn codegen_instance(inst: &InstantiatedModule, md: &Module, out_file: &mut File) write!(out_file, "// {inst_name}\n{code}").unwrap(); } -fn make_output_file(name : &str) -> File { - let mut path = PathBuf::with_capacity(name.len()+"verilog_output/.sv".len()); +fn make_output_file(name: &str) -> File { + let mut path = PathBuf::with_capacity(name.len() + "verilog_output/.sv".len()); path.push("verilog_output"); fs::create_dir_all(&path).unwrap(); path.push(name); path.set_extension("sv"); let mut file = File::create(path).unwrap(); - file.write_fmt(format_args!("// DO NOT EDIT THIS FILE\n// This file was generated with SUS Compiler {}", std::env!("CARGO_PKG_VERSION"))).unwrap(); - + file.write_fmt(format_args!( + "// DO NOT EDIT THIS FILE\n// This file was generated with SUS Compiler {}", + std::env!("CARGO_PKG_VERSION") + )) + .unwrap(); + file } @@ -106,10 +110,10 @@ fn codegen_with_dependencies(linker: &Linker, md: &Module, file_name: &str) { } fn main() -> Result<(), Box> { - let file_paths = parse_args(); - let config = config(); + let file_paths = config.files.clone(); + if config.use_lsp { #[cfg(feature = "lsp")] return dev_aid::lsp::lsp_main(); @@ -121,7 +125,9 @@ fn main() -> Result<(), Box> { let (linker, mut paths_arena) = compile_all(file_paths); print_all_errors(&linker, &mut paths_arena.file_sources); - if config.early_exit != EarlyExitUpTo::CodeGen {return Ok(())} + if config.early_exit != EarlyExitUpTo::CodeGen { + return Ok(()); + } if config.codegen { for (_id, md) in &linker.modules { @@ -133,8 +139,8 @@ fn main() -> Result<(), Box> { let Some(md) = linker .modules .iter() - .find(|(_, md)| &md.link_info.name == md_name) else { - + .find(|(_, md)| &md.link_info.name == md_name) + else { let mut err_lock = std::io::stderr().lock(); writeln!(err_lock, "Unknown module {md_name}").unwrap(); std::process::exit(1); From 8a27203e3144c7433d6cdc711e747146f2524c5b Mon Sep 17 00:00:00 2001 From: Tobias Tom Hodapp Date: Tue, 19 Nov 2024 15:12:57 +0100 Subject: [PATCH 2/3] Bumped rust version to 1.80 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 39d2be6..fc59066 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ homepage = "https://github.com/pc2/sus-compiler" readme = "README.md" keywords = ["sus", "fpga", "vlsi", "hdl", "verilog"] categories = ["compilers", "text-processing"] -rust-version = "1.78" +rust-version = "1.80" include = ["/src", "/stl/*", "/README.md", "/LICENSE", "/CHANGELOG.md", "/build.rs", "/rustfmt.toml"] From 601ad5374c9fbee51e760177948c85f356083e80 Mon Sep 17 00:00:00 2001 From: Tobias Tom Hodapp Date: Tue, 19 Nov 2024 15:22:29 +0100 Subject: [PATCH 3/3] Removed useless collect --- src/config.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/config.rs b/src/config.rs index 145029a..7446b69 100644 --- a/src/config.rs +++ b/src/config.rs @@ -156,8 +156,7 @@ where pub fn config() -> &'static ConfigStruct { static CONFIG: LazyLock = LazyLock::new(|| { - let args: Vec = std::env::args_os().collect(); - parse_args(args.as_slice()) + parse_args(std::env::args_os()) .map_err(|err| err.exit()) .unwrap() });