Merge #125

125: Small maintainance improvements in cargo and rustc r=Niki4tap a=xFrednet

This PR does some cleanup around the rustc driver and cargo. The commits should structure the changes pretty well.

---

I took a break after my exam week. It has been pretty good, but now I want and need to get back into marker. I hope that this PR is just the start of it :)

---

r? `@Niki4tap` Would you still have time to review my PRs? 🙃 

Co-authored-by: xFrednet <xFrednet@gmail.com>

.cargo/config.toml

@@ -1,3 +1,6 @@
 [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]
+split-debuginfo = "unpacked"

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())

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")

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<PathBuf, ExitStatus> {
-        build_local_lint_crate(self, target_dir, verbose)
+    pub fn build(&self, target_dir: &Path, flags: &Flags) -> Result<PathBuf, ExitStatus> {
+        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<PathBuf, ExitStatus> {
+fn build_local_lint_crate(krate: &LintCrateSpec<'_>, target_dir: &Path, flags: &Flags) -> Result<PathBuf, ExitStatus> {
     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()

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)
     }
 }
 

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) => {