From cd0f5217b0fe94fce57cf8334a9a21f45397d900 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Wed, 22 Mar 2023 23:50:42 +0100 Subject: [PATCH 1/3] Set `split-debuginfo = unpacked` for faster compile on linux --- .cargo/config.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.cargo/config.toml b/.cargo/config.toml index 3cf7359d..aa8c1584 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,3 +1,6 @@ [alias] dogfood = "run --features dev-build --bin cargo-marker" uitest = "test --test compile_test" + +[profile.dev] +split-debuginfo = "unpacked" From 4a73c6c1c8ebda5fbb82d15a172a12f06f14e915 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Thu, 23 Mar 2023 00:12:31 +0100 Subject: [PATCH 2/3] Rustc: Unify id conversion layouts --- marker_driver_rustc/src/conversion/common.rs | 30 ------ .../src/conversion/marker/common.rs | 98 +++++++------------ .../src/conversion/rustc/common.rs | 31 +++--- 3 files changed, 47 insertions(+), 112 deletions(-) diff --git a/marker_driver_rustc/src/conversion/common.rs b/marker_driver_rustc/src/conversion/common.rs index 79ff8866..b4623f07 100644 --- a/marker_driver_rustc/src/conversion/common.rs +++ b/marker_driver_rustc/src/conversion/common.rs @@ -1,27 +1,9 @@ -#[repr(C)] -pub struct GenericIdLayout { - pub krate: u32, - pub index: u32, -} - -#[repr(C)] -pub struct TyDefIdLayout { - pub krate: u32, - pub index: u32, -} - #[repr(C)] pub struct DefIdLayout { pub krate: u32, pub index: u32, } -#[repr(C)] -pub struct ItemIdLayout { - pub krate: u32, - pub index: u32, -} - #[repr(C)] pub struct BodyIdLayout { // Note: AFAIK rustc only loads bodies from the current crate, this allows @@ -39,12 +21,6 @@ pub struct DefIdInfo { pub krate: u32, } -#[repr(C)] -pub struct ExprIdLayout { - pub owner: u32, - pub index: u32, -} - #[repr(C)] pub struct HirIdLayout { pub owner: u32, @@ -57,12 +33,6 @@ pub struct SpanSourceInfo { pub rustc_start_offset: usize, } -#[repr(C)] -pub struct VarIdLayout { - pub owner: u32, - pub index: u32, -} - #[macro_export] macro_rules! transmute_id { ($t1:ty as $t2:ty = $e:expr) => { diff --git a/marker_driver_rustc/src/conversion/marker/common.rs b/marker_driver_rustc/src/conversion/marker/common.rs index 2e57f964..a7983194 100644 --- a/marker_driver_rustc/src/conversion/marker/common.rs +++ b/marker_driver_rustc/src/conversion/marker/common.rs @@ -9,49 +9,42 @@ use marker_api::lint::Level; use rustc_hir as hir; use rustc_middle as mid; -use crate::conversion::common::{ - BodyIdLayout, ExprIdLayout, GenericIdLayout, HirIdLayout, ItemIdLayout, SpanSourceInfo, TyDefIdLayout, VarIdLayout, -}; +use crate::conversion::common::{BodyIdLayout, DefIdLayout, HirIdLayout, SpanSourceInfo}; use crate::transmute_id; use super::MarkerConverterInner; -impl From for GenericIdLayout { +impl From for DefIdLayout { fn from(value: hir::def_id::LocalDefId) -> Self { value.to_def_id().into() } } -impl From for GenericIdLayout { - fn from(rustc_id: hir::def_id::DefId) -> Self { - GenericIdLayout { - krate: rustc_id.krate.as_u32(), - index: rustc_id.index.as_u32(), - } - } -} - -impl From for ItemIdLayout { +impl From for DefIdLayout { fn from(value: hir::ItemId) -> Self { // My understanding is, that the `owner_id` is the `DefId` of this item. // We'll see if this holds true, when marker crashes and burns ^^ value.owner_id.def_id.into() } } -impl From for ItemIdLayout { - fn from(value: hir::def_id::LocalDefId) -> Self { +impl From for DefIdLayout { + fn from(value: hir::OwnerId) -> Self { value.to_def_id().into() } } -impl From for ItemIdLayout { - fn from(value: hir::OwnerId) -> Self { - value.to_def_id().into() +impl From for DefIdLayout { + fn from(value: hir::def_id::DefId) -> Self { + DefIdLayout { + krate: value.krate.as_u32(), + index: value.index.as_u32(), + } } } -impl From for ItemIdLayout { - fn from(rustc_id: hir::def_id::DefId) -> Self { - ItemIdLayout { - krate: rustc_id.krate.as_u32(), - index: rustc_id.index.as_u32(), + +impl From for HirIdLayout { + fn from(value: hir::HirId) -> Self { + HirIdLayout { + owner: value.owner.def_id.local_def_index.as_u32(), + index: value.local_id.as_u32(), } } } @@ -91,37 +84,27 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> { } #[must_use] - pub fn to_generic_id(&self, id: impl Into) -> GenericId { - transmute_id!(GenericIdLayout as GenericId = id.into()) + pub fn to_generic_id(&self, id: impl Into) -> GenericId { + transmute_id!(DefIdLayout as GenericId = id.into()) } #[must_use] #[expect(dead_code, reason = "will be used later")] - pub fn to_ty_def_id(&self, rustc_id: hir::def_id::DefId) -> TyDefId { - transmute_id!( - TyDefIdLayout as TyDefId = TyDefIdLayout { - krate: rustc_id.krate.as_u32(), - index: rustc_id.index.as_u32(), - } - ) + pub fn to_ty_def_id(&self, id: impl Into) -> TyDefId { + transmute_id!(DefIdLayout as TyDefId = id.into()) } - pub fn to_item_id(&self, id: impl Into) -> ItemId { - transmute_id!(ItemIdLayout as ItemId = id.into()) + pub fn to_item_id(&self, id: impl Into) -> ItemId { + transmute_id!(DefIdLayout as ItemId = id.into()) } - pub fn to_variant_id(&self, id: impl Into) -> VariantId { - transmute_id!(ItemIdLayout as VariantId = id.into()) + pub fn to_variant_id(&self, id: impl Into) -> VariantId { + transmute_id!(DefIdLayout as VariantId = id.into()) } #[must_use] - pub fn to_field_id(&self, rustc_id: hir::HirId) -> FieldId { - transmute_id!( - HirIdLayout as FieldId = HirIdLayout { - owner: rustc_id.owner.def_id.local_def_index.as_u32(), - index: rustc_id.local_id.as_u32(), - } - ) + pub fn to_field_id(&self, id: impl Into) -> FieldId { + transmute_id!(HirIdLayout as FieldId = id.into()) } #[must_use] pub fn to_body_id(&self, rustc_id: hir::BodyId) -> BodyId { @@ -134,33 +117,18 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> { } #[must_use] - pub fn to_var_id(&self, rustc_id: hir::HirId) -> VarId { - transmute_id!( - VarIdLayout as VarId = VarIdLayout { - owner: rustc_id.owner.def_id.local_def_index.as_u32(), - index: rustc_id.local_id.as_u32(), - } - ) + pub fn to_var_id(&self, id: impl Into) -> VarId { + transmute_id!(HirIdLayout as VarId = id.into()) } #[must_use] - pub fn to_expr_id(&self, rustc_id: hir::HirId) -> ExprId { - transmute_id!( - ExprIdLayout as ExprId = ExprIdLayout { - owner: rustc_id.owner.def_id.local_def_index.as_u32(), - index: rustc_id.local_id.as_u32(), - } - ) + pub fn to_expr_id(&self, id: impl Into) -> ExprId { + transmute_id!(HirIdLayout as ExprId = id.into()) } #[must_use] - pub fn to_let_stmt_id(&self, rustc_id: hir::HirId) -> LetStmtId { - transmute_id!( - HirIdLayout as LetStmtId = HirIdLayout { - owner: rustc_id.owner.def_id.local_def_index.as_u32(), - index: rustc_id.local_id.as_u32(), - } - ) + pub fn to_let_stmt_id(&self, id: impl Into) -> LetStmtId { + transmute_id!(HirIdLayout as LetStmtId = id.into()) } } diff --git a/marker_driver_rustc/src/conversion/rustc/common.rs b/marker_driver_rustc/src/conversion/rustc/common.rs index f5cb4371..1582bcc4 100644 --- a/marker_driver_rustc/src/conversion/rustc/common.rs +++ b/marker_driver_rustc/src/conversion/rustc/common.rs @@ -10,19 +10,16 @@ use marker_api::{ }; use rustc_hir as hir; -use crate::conversion::common::{ - BodyIdLayout, DefIdInfo, DefIdLayout, ExprIdLayout, GenericIdLayout, HirIdLayout, ItemIdLayout, TyDefIdLayout, - VarIdLayout, -}; +use crate::conversion::common::{BodyIdLayout, DefIdInfo, DefIdLayout, HirIdLayout}; use crate::transmute_id; use super::RustcConverter; macro_rules! impl_into_def_id_for { - ($id:ty, $layout:ty) => { + ($id:ty) => { impl From<$id> for DefIdInfo { fn from(value: $id) -> Self { - let layout = transmute_id!($id as $layout = value); + let layout = transmute_id!($id as DefIdLayout = value); DefIdInfo { index: layout.index, krate: layout.krate, @@ -32,10 +29,10 @@ macro_rules! impl_into_def_id_for { }; } -impl_into_def_id_for!(GenericId, GenericIdLayout); -impl_into_def_id_for!(ItemId, ItemIdLayout); -impl_into_def_id_for!(TyDefId, TyDefIdLayout); -impl_into_def_id_for!(VariantId, DefIdLayout); +impl_into_def_id_for!(GenericId); +impl_into_def_id_for!(ItemId); +impl_into_def_id_for!(TyDefId); +impl_into_def_id_for!(VariantId); pub struct HirIdInfo { pub owner: u32, @@ -43,10 +40,10 @@ pub struct HirIdInfo { } macro_rules! impl_into_hir_id_for { - ($id:ty, $layout:ty) => { + ($id:ty) => { impl From<$id> for HirIdInfo { fn from(value: $id) -> Self { - let layout = transmute_id!($id as $layout = value); + let layout = transmute_id!($id as HirIdLayout = value); HirIdInfo { owner: layout.owner, index: layout.index, @@ -56,10 +53,10 @@ macro_rules! impl_into_hir_id_for { }; } -impl_into_hir_id_for!(ExprId, ExprIdLayout); -impl_into_hir_id_for!(VarId, VarIdLayout); -impl_into_hir_id_for!(LetStmtId, HirIdLayout); -impl_into_hir_id_for!(FieldId, HirIdLayout); +impl_into_hir_id_for!(ExprId); +impl_into_hir_id_for!(VarId); +impl_into_hir_id_for!(LetStmtId); +impl_into_hir_id_for!(FieldId); #[derive(Debug, Clone, Copy)] pub struct SpanSourceInfo { @@ -76,7 +73,7 @@ impl<'ast, 'tcx> RustcConverter<'ast, 'tcx> { #[must_use] pub fn to_item_id(&self, api_id: ItemId) -> hir::ItemId { - let layout = transmute_id!(ItemId as ItemIdLayout = api_id); + let layout = transmute_id!(ItemId as DefIdLayout = api_id); hir::ItemId { owner_id: hir::OwnerId { def_id: hir::def_id::LocalDefId { From a7e4ea1d883af79b3e064c08216b14586b42d8bb Mon Sep 17 00:00:00 2001 From: xFrednet Date: Fri, 10 Mar 2023 17:48:06 +0100 Subject: [PATCH 3/3] Cargo: Add `--forward-rust-flags` to forward `RUSTFLAGS` env --- .cargo/config.toml | 2 +- cargo-marker/src/cli.rs | 27 ++++++++++++++++++++++++- cargo-marker/src/driver.rs | 27 ++++++++++++++++--------- cargo-marker/src/lints.rs | 19 ++++++++++++------ cargo-marker/src/main.rs | 41 +++++++++++--------------------------- 5 files changed, 70 insertions(+), 46 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index aa8c1584..dc606b59 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,5 +1,5 @@ [alias] -dogfood = "run --features dev-build --bin cargo-marker" +dogfood = "run --features dev-build --bin cargo-marker -- --forward-rust-flags" uitest = "test --test compile_test" [profile.dev] diff --git a/cargo-marker/src/cli.rs b/cargo-marker/src/cli.rs index 32e87c90..5503319b 100644 --- a/cargo-marker/src/cli.rs +++ b/cargo-marker/src/cli.rs @@ -1,4 +1,4 @@ -use clap::{builder::ValueParser, Arg, ArgAction, Command}; +use clap::{builder::ValueParser, Arg, ArgAction, ArgMatches, Command}; use crate::VERSION; @@ -10,6 +10,25 @@ EXAMPLES: * `cargo marker -l ./marker_lints` "#; +#[allow(clippy::struct_excessive_bools)] +pub struct Flags { + pub verbose: bool, + pub test_build: bool, + pub dev_build: bool, + pub forward_rust_flags: bool, +} + +impl Flags { + pub fn from_args(args: &ArgMatches) -> Self { + Self { + verbose: args.get_flag("verbose"), + test_build: args.get_flag("test-setup"), + dev_build: cfg!(feature = "dev-build"), + forward_rust_flags: args.get_flag("forward-rust-flags"), + } + } +} + pub fn get_clap_config() -> Command { Command::new(VERSION) .arg( @@ -32,6 +51,12 @@ pub fn get_clap_config() -> Command { .action(ArgAction::SetTrue) .help("This flag will compile the lint crate and print all relevant environment values"), ) + .arg( + Arg::new("forward-rust-flags") + .long("forward-rust-flags") + .action(ArgAction::SetTrue) + .help("Forwards the current `RUSTFLAGS` value during driver and lint compilation"), + ) .subcommand(setup_command()) .subcommand(check_command()) .args(check_command_args()) diff --git a/cargo-marker/src/driver.rs b/cargo-marker/src/driver.rs index 93d09974..93326625 100644 --- a/cargo-marker/src/driver.rs +++ b/cargo-marker/src/driver.rs @@ -16,7 +16,7 @@ use std::{ffi::OsString, path::PathBuf, process::Command}; use once_cell::sync::Lazy; -use crate::ExitStatus; +use crate::{cli::Flags, ExitStatus}; /// This is the driver version and toolchain, that is used by the setup command /// to install the driver. @@ -41,7 +41,7 @@ pub fn print_driver_version() { } /// This tries to install the rustc driver specified in [`DEFAULT_DRIVER_INFO`]. -pub fn install_driver(verbose: bool, dev_build: bool) -> Result<(), ExitStatus> { +pub fn install_driver(flags: &Flags) -> Result<(), ExitStatus> { // The toolchain, driver version and api version should ideally be configurable. // However, that will require more prototyping and has a low priority rn. // See #60 @@ -50,10 +50,10 @@ pub fn install_driver(verbose: bool, dev_build: bool) -> Result<(), ExitStatus> let toolchain = &DEFAULT_DRIVER_INFO.toolchain; check_toolchain(toolchain)?; - build_driver(toolchain, &DEFAULT_DRIVER_INFO.version, verbose, dev_build)?; + build_driver(toolchain, &DEFAULT_DRIVER_INFO.version, flags)?; // We don't want to advice the user, to install the driver again. - check_driver(verbose, false) + check_driver(flags.verbose, false) } /// This function checks if the specified toolchain is installed. This requires @@ -73,8 +73,8 @@ fn check_toolchain(toolchain: &str) -> Result<(), ExitStatus> { /// This tries to compile the driver. If successful the driver binary will /// be places next to the executable of `cargo-linter`. -fn build_driver(toolchain: &str, version: &str, verbose: bool, dev_build: bool) -> Result<(), ExitStatus> { - if dev_build { +fn build_driver(toolchain: &str, version: &str, flags: &Flags) -> Result<(), ExitStatus> { + if flags.dev_build { println!("Compiling rustc driver"); } else { println!("Compiling rustc driver v{version} with {toolchain}"); @@ -83,15 +83,21 @@ fn build_driver(toolchain: &str, version: &str, verbose: bool, dev_build: bool) // Build driver let mut cmd = Command::new("cargo"); - if !dev_build { + if !flags.dev_build { cmd.arg(&format!("+{toolchain}")); } - if verbose { + if flags.verbose { cmd.arg("--verbose"); } - if dev_build { + let mut rustc_flags = if flags.forward_rust_flags { + std::env::var("RUSTFLAGS").unwrap_or_default() + } else { + String::new() + }; + + if flags.dev_build { cmd.args(["build", "--bin", "marker_driver_rustc"]); } else { // FIXME: This currently installs the binary in Cargo's default location. @@ -105,8 +111,11 @@ fn build_driver(toolchain: &str, version: &str, verbose: bool, dev_build: bool) // // See #60 cmd.args(["install", "marker_rustc_driver", "--version", version]); + rustc_flags += " --cap-lints=allow"; } + cmd.env("RUSTFLAGS", rustc_flags); + // `RUSTFLAGS` is never set for driver compilation, we might want to let status = cmd .spawn() .expect("unable to start cargo install for the driver") diff --git a/cargo-marker/src/lints.rs b/cargo-marker/src/lints.rs index 60fc1a4f..5abbe240 100644 --- a/cargo-marker/src/lints.rs +++ b/cargo-marker/src/lints.rs @@ -4,7 +4,7 @@ use std::{ process::Command, }; -use crate::ExitStatus; +use crate::{cli::Flags, ExitStatus}; pub struct LintCrateSpec<'a> { /// Optional package name (this is always UTF-8, as opposed to `dir`), exists if supplied from @@ -42,23 +42,30 @@ impl<'a> LintCrateSpec<'a> { /// Creates a debug build for this crate. The path of the build library /// will be returned, if the operation was successful. - pub fn build(&self, target_dir: &Path, verbose: bool) -> Result { - build_local_lint_crate(self, target_dir, verbose) + pub fn build(&self, target_dir: &Path, flags: &Flags) -> Result { + build_local_lint_crate(self, target_dir, flags) } } /// This creates a debug build for a local crate. The path of the build library /// will be returned, if the operation was successful. -fn build_local_lint_crate(krate: &LintCrateSpec<'_>, target_dir: &Path, verbose: bool) -> Result { +fn build_local_lint_crate(krate: &LintCrateSpec<'_>, target_dir: &Path, flags: &Flags) -> Result { if !krate.dir.exists() { eprintln!("The given lint can't be found, searched at: `{}`", krate.dir.display()); return Err(ExitStatus::LintCrateNotFound); } + let mut rustc_flags = if flags.forward_rust_flags { + std::env::var("RUSTFLAGS").unwrap_or_default() + } else { + String::new() + }; + rustc_flags += " --cap-lints=allow"; + // Compile the lint crate let mut cmd = Command::new("cargo"); cmd.arg("build"); - if verbose { + if flags.verbose { cmd.arg("--verbose"); } if let Some(name) = krate.package_name { @@ -69,7 +76,7 @@ fn build_local_lint_crate(krate: &LintCrateSpec<'_>, target_dir: &Path, verbose: .current_dir(std::fs::canonicalize(krate.dir).unwrap()) .args(["--lib", "--target-dir"]) .arg(target_dir.as_os_str()) - .env("RUSTFLAGS", "--cap-lints=allow") + .env("RUSTFLAGS", rustc_flags) .spawn() .expect("could not run cargo") .wait() diff --git a/cargo-marker/src/main.rs b/cargo-marker/src/main.rs index 615fc294..44564666 100644 --- a/cargo-marker/src/main.rs +++ b/cargo-marker/src/main.rs @@ -15,7 +15,7 @@ use std::{ process::exit, }; -use cli::get_clap_config; +use cli::{get_clap_config, Flags}; use config::Config; use driver::{get_driver_path, run_driver}; use lints::LintCrateSpec; @@ -109,12 +109,10 @@ fn main() -> Result<(), ExitStatus> { .take_while(|s| s != CARGO_ARGS_SEPARATOR), ); - let verbose = matches.get_flag("verbose"); - let test_build = matches.get_flag("test-setup"); - let dev_build = cfg!(feature = "dev-build"); + let flags = Flags::from_args(&matches); if matches.get_flag("version") { - print_version(verbose); + print_version(flags.verbose); return Ok(()); } @@ -127,32 +125,17 @@ fn main() -> Result<(), ExitStatus> { }; match matches.subcommand() { - Some(("setup", _args)) => driver::install_driver(verbose, dev_build), - Some(("check", args)) => run_check( - &choose_lint_crates(args, config.as_ref())?, - verbose, - dev_build, - test_build, - ), - None => run_check( - &choose_lint_crates(&matches, config.as_ref())?, - verbose, - dev_build, - test_build, - ), + Some(("setup", _args)) => driver::install_driver(&flags), + Some(("check", args)) => run_check(&choose_lint_crates(args, config.as_ref())?, &flags), + None => run_check(&choose_lint_crates(&matches, config.as_ref())?, &flags), _ => unreachable!(), } } -fn run_check( - crate_entries: &[LintCrateSpec], - verbose: bool, - dev_build: bool, - test_build: bool, -) -> Result<(), ExitStatus> { +fn run_check(crate_entries: &[LintCrateSpec], flags: &Flags) -> Result<(), ExitStatus> { // If this is a dev build, we want to recompile the driver before checking - if dev_build { - driver::install_driver(verbose, dev_build)?; + if flags.dev_build { + driver::install_driver(flags)?; } if crate_entries.is_empty() { @@ -171,7 +154,7 @@ fn run_check( println!("Compiling Lints:"); let target_dir = Path::new(&*MARKER_LINT_DIR); for krate in crate_entries { - let crate_file = krate.build(target_dir, verbose)?; + let crate_file = krate.build(target_dir, flags)?; lint_crates.push(crate_file.as_os_str().to_os_string()); } @@ -180,12 +163,12 @@ fn run_check( (OsString::from("RUSTC_WORKSPACE_WRAPPER"), get_driver_path().as_os_str().to_os_string()), (OsString::from("MARKER_LINT_CRATES"), lint_crates.join(OsStr::new(";"))) ]; - if test_build { + if flags.test_build { print_env(env).unwrap(); Ok(()) } else { let cargo_args = std::env::args().skip_while(|c| c != CARGO_ARGS_SEPARATOR).skip(1); - run_driver(env, cargo_args, verbose) + run_driver(env, cargo_args, flags.verbose) } }