Merge pull request #1724 from blyxyas/clippy-timer

Integrate Clippy into the project

Author: Kobzol

collector/benchlib/src/benchmark.rs

@@ -119,8 +119,15 @@ impl<'a> BenchmarkGroup<'a> {
 
     fn profile_benchmark(self, args: ProfileArgs) -> anyhow::Result<()> {
         let Some(benchmark) = self.benchmarks.get(args.benchmark.as_str()) else {
-            return Err(anyhow::anyhow!("Benchmark `{}` not found. Available benchmarks: {}", args.benchmark,
-                self.benchmarks.keys().map(|s| s.to_string()).collect::<Vec<_>>().join(", ")));
+            return Err(anyhow::anyhow!(
+                "Benchmark `{}` not found. Available benchmarks: {}",
+                args.benchmark,
+                self.benchmarks
+                    .keys()
+                    .map(|s| s.to_string())
+                    .collect::<Vec<_>>()
+                    .join(", ")
+            ));
         };
         (benchmark.profile_fn)();
 

collector/src/bin/collector.rs

@@ -39,6 +39,7 @@ use collector::runtime::{
 use collector::runtime::{profile_runtime, RuntimeCompilationOpts};
 use collector::toolchain::{
     create_toolchain_from_published_version, get_local_toolchain, Sysroot, Toolchain,
+    ToolchainConfig,
 };
 use collector::utils::cachegrind::cachegrind_diff;
 use collector::utils::{is_installed, wait_for_future};
@@ -375,6 +376,10 @@ struct CompileTimeOptions {
     /// The path to the local rustdoc to measure
     #[arg(long)]
     rustdoc: Option<PathBuf>,
+
+    /// The path to the local clippy to measure
+    #[arg(long)]
+    clippy: Option<PathBuf>,
 }
 
 #[derive(Debug, clap::Args)]
@@ -660,9 +665,7 @@ fn main_result() -> anyhow::Result<i32> {
                 let toolchain = get_local_toolchain(
                     &[Profile::Opt],
                     rustc,
-                    None,
-                    None,
-                    None,
+                    ToolchainConfig::default(),
                     id,
                     target_triple.clone(),
                 )?;
@@ -718,9 +721,7 @@ fn main_result() -> anyhow::Result<i32> {
                 let toolchain = get_local_toolchain(
                     &[Profile::Opt],
                     rustc,
-                    None,
-                    None,
-                    None,
+                    ToolchainConfig::default(),
                     id,
                     target_triple.clone(),
                 )?;
@@ -756,9 +757,11 @@ fn main_result() -> anyhow::Result<i32> {
             let toolchain = get_local_toolchain(
                 &profiles,
                 &local.rustc,
-                opts.rustdoc.as_deref(),
-                local.cargo.as_deref(),
-                local.id.as_deref(),
+                *ToolchainConfig::default()
+                    .rustdoc(opts.rustdoc.as_deref())
+                    .clippy(opts.clippy.as_deref())
+                    .cargo(local.cargo.as_deref())
+                    .id(local.id.as_deref()),
                 "",
                 target_triple,
             )?;
@@ -948,9 +951,11 @@ fn main_result() -> anyhow::Result<i32> {
                     let toolchain = get_local_toolchain(
                         profiles,
                         rustc,
-                        opts.rustdoc.as_deref(),
-                        local.cargo.as_deref(),
-                        local.id.as_deref(),
+                        *ToolchainConfig::default()
+                            .rustdoc(opts.rustdoc.as_deref())
+                            .clippy(opts.clippy.as_deref())
+                            .cargo(local.cargo.as_deref())
+                            .id(local.id.as_deref()),
                         suffix,
                         target_triple.clone(),
                     )?;
@@ -1062,9 +1067,9 @@ fn get_local_toolchain_for_runtime_benchmarks(
     get_local_toolchain(
         &[Profile::Opt],
         &local.rustc,
-        None,
-        local.cargo.as_deref(),
-        local.id.as_deref(),
+        *ToolchainConfig::default()
+            .cargo(local.cargo.as_deref())
+            .id(local.id.as_deref()),
         "",
         target_triple.to_string(),
     )
@@ -1193,9 +1198,9 @@ fn bench_published_artifact(
     let artifact_id = ArtifactId::Tag(toolchain.id.clone());
 
     let profiles = if collector::version_supports_doc(&toolchain.id) {
-        Profile::all()
+        vec![Profile::Check, Profile::Debug, Profile::Doc, Profile::Opt]
     } else {
-        Profile::all_non_doc()
+        vec![Profile::Check, Profile::Debug, Profile::Opt]
     };
     let scenarios = if collector::version_supports_incremental(&toolchain.id) {
         Scenario::all()

collector/src/bin/rustc-fake.rs

@@ -35,8 +35,11 @@ fn main() {
     let mut args = args_os.collect::<Vec<_>>();
     let rustc = env::var_os("RUSTC_REAL").unwrap();
     let actually_rustdoc = name.ends_with("rustdoc-fake");
+    let actually_clippy = name.ends_with("clippy-fake");
     let tool = if actually_rustdoc {
         env::var_os("RUSTDOC_REAL").unwrap()
+    } else if actually_clippy {
+        env::var_os("CLIPPY_REAL").unwrap()
     } else {
         rustc
     };

collector/src/compile/benchmark/profile.rs

@@ -12,13 +12,15 @@ pub enum Profile {
     Debug,
     Doc,
     Opt,
+    Clippy,
 }
 
 impl Profile {
     pub fn all() -> Vec<Self> {
         vec![Profile::Check, Profile::Debug, Profile::Doc, Profile::Opt]
     }
 
+    // This also leaves Clippy out
     pub fn all_non_doc() -> Vec<Self> {
         vec![Profile::Check, Profile::Debug, Profile::Opt]
     }

collector/src/compile/execute/bencher.rs

@@ -87,6 +87,7 @@ impl<'a> BenchProcessor<'a> {
             Profile::Debug => database::Profile::Debug,
             Profile::Doc => database::Profile::Doc,
             Profile::Opt => database::Profile::Opt,
+            Profile::Clippy => database::Profile::Clippy,
         };
 
         if let Some(files) = stats.2 {

collector/src/compile/execute/mod.rs

@@ -72,7 +72,7 @@ impl PerfTool {
             }
             ProfileTool(LlvmLines) => match profile {
                 Profile::Debug | Profile::Opt => Some("llvm-lines"),
-                Profile::Check | Profile::Doc => None,
+                Profile::Check | Profile::Doc | Profile::Clippy => None,
             },
         }
     }
@@ -168,6 +168,10 @@ impl<'a> CargoProcess<'a> {
 
         if let Some(r) = &self.toolchain.components.rustdoc {
             cmd.env("RUSTDOC", &*FAKE_RUSTDOC).env("RUSTDOC_REAL", r);
+        };
+
+        if let Some(c) = &self.toolchain.components.clippy {
+            cmd.env("CLIPPY", &*FAKE_CLIPPY).env("CLIPPY_REAL", c);
         }
         cmd
     }
@@ -236,6 +240,7 @@ impl<'a> CargoProcess<'a> {
                 }
                 Profile::Debug => {}
                 Profile::Doc => {}
+                Profile::Clippy => {}
                 Profile::Opt => {
                     cmd.arg("--release");
                 }
@@ -354,6 +359,21 @@ lazy_static::lazy_static! {
         }
         fake_rustdoc
     };
+    static ref FAKE_CLIPPY: PathBuf = {
+        let mut fake_clippy = env::current_exe().unwrap();
+        fake_clippy.pop();
+        fake_clippy.push("clippy-fake");
+        // link from rustc-fake to rustdoc-fake
+        if !fake_clippy.exists() {
+            #[cfg(unix)]
+            use std::os::unix::fs::symlink;
+            #[cfg(windows)]
+            use std::os::windows::fs::symlink_file as symlink;
+
+            symlink(&*FAKE_RUSTC, &fake_clippy).expect("failed to make symbolic link");
+        }
+        fake_clippy
+    };
 }
 
 /// Used to indicate if we need to retry a run.

collector/src/toolchain.rs

@@ -123,6 +123,7 @@ impl SysrootDownload {
         let components = ToolchainComponents::from_binaries_and_libdir(
             sysroot_bin("rustc")?,
             Some(sysroot_bin("rustdoc")?),
+            sysroot_bin("cargo-clippy").ok(),
             sysroot_bin("cargo")?,
             &self.directory.join(&self.rust_sha).join("lib"),
         )?;
@@ -241,6 +242,7 @@ impl Toolchain {
 pub struct ToolchainComponents {
     pub rustc: PathBuf,
     pub rustdoc: Option<PathBuf>,
+    pub clippy: Option<PathBuf>,
     pub cargo: PathBuf,
     pub lib_rustc: Option<PathBuf>,
     pub lib_std: Option<PathBuf>,
@@ -252,12 +254,14 @@ impl ToolchainComponents {
     fn from_binaries_and_libdir(
         rustc: PathBuf,
         rustdoc: Option<PathBuf>,
+        clippy: Option<PathBuf>,
         cargo: PathBuf,
         libdir: &Path,
     ) -> anyhow::Result<Self> {
         let mut component = ToolchainComponents {
             rustc,
             rustdoc,
+            clippy,
             cargo,
             ..Default::default()
         };
@@ -288,6 +292,36 @@ impl ToolchainComponents {
     }
 }
 
+#[derive(Clone, Copy, Default)]
+pub struct ToolchainConfig<'a> {
+    rustdoc: Option<&'a Path>,
+    clippy: Option<&'a Path>,
+    cargo: Option<&'a Path>,
+    id: Option<&'a str>,
+}
+
+impl<'a> ToolchainConfig<'a> {
+    pub fn rustdoc(&mut self, rustdoc: Option<&'a Path>) -> &mut Self {
+        self.rustdoc = rustdoc;
+        self
+    }
+
+    pub fn clippy(&mut self, clippy: Option<&'a Path>) -> &mut Self {
+        self.clippy = clippy;
+        self
+    }
+
+    pub fn cargo(&mut self, cargo: Option<&'a Path>) -> &mut Self {
+        self.cargo = cargo;
+        self
+    }
+
+    pub fn id(&mut self, id: Option<&'a str>) -> &mut Self {
+        self.id = id;
+        self
+    }
+}
+
 /// Get a toolchain from the input.
 /// - `rustc`: check if the given one is acceptable.
 /// - `rustdoc`: if one is given, check if it is acceptable. Otherwise, if
@@ -297,9 +331,7 @@ impl ToolchainComponents {
 pub fn get_local_toolchain(
     profiles: &[Profile],
     rustc: &str,
-    rustdoc: Option<&Path>,
-    cargo: Option<&Path>,
-    id: Option<&str>,
+    toolchain_config: ToolchainConfig<'_>,
     id_suffix: &str,
     target_triple: String,
 ) -> anyhow::Result<Toolchain> {
@@ -354,7 +386,7 @@ pub fn get_local_toolchain(
         debug!("found rustc: {:?}", &rustc);
 
         // When the id comes from a +toolchain, the suffix is *not* added.
-        let id = if let Some(id) = id {
+        let id = if let Some(id) = toolchain_config.id {
             let mut id = id.to_owned();
             id.push_str(id_suffix);
             id
@@ -369,7 +401,7 @@ pub fn get_local_toolchain(
 
         // When specifying rustc via a path, the suffix is always added to the
         // id.
-        let mut id = if let Some(id) = id {
+        let mut id = if let Some(id) = toolchain_config.id {
             id.to_owned()
         } else {
             "Id".to_string()
@@ -380,7 +412,7 @@ pub fn get_local_toolchain(
     };
 
     let rustdoc =
-        if let Some(rustdoc) = &rustdoc {
+        if let Some(rustdoc) = &toolchain_config.rustdoc {
             Some(rustdoc.canonicalize().with_context(|| {
                 format!("failed to canonicalize rustdoc executable {:?}", rustdoc)
             })?)
@@ -400,7 +432,28 @@ pub fn get_local_toolchain(
             None
         };
 
-    let cargo = if let Some(cargo) = &cargo {
+    let clippy = if let Some(clippy) = &toolchain_config.clippy {
+        Some(
+            clippy.canonicalize().with_context(|| {
+                format!("failed to canonicalize clippy executable {:?}", clippy)
+            })?,
+        )
+    } else if profiles.contains(&Profile::Clippy) {
+        // We need a `clippy`. Look for one next to `rustc`.
+        if let Ok(clippy) = rustc.with_file_name("cargo-clippy").canonicalize() {
+            debug!("found clippy: {:?}", &clippy);
+            Some(clippy)
+        } else {
+            anyhow::bail!(
+                    "'Clippy' build specified but '--cargo-clippy' not specified and no 'cargo-clippy' found \
+                    next to 'rustc'"
+                );
+        }
+    } else {
+        // No `clippy` provided, but none needed.
+        None
+    };
+    let cargo = if let Some(cargo) = &toolchain_config.cargo {
         cargo
             .canonicalize()
             .with_context(|| format!("failed to canonicalize cargo executable {:?}", cargo))?
@@ -428,7 +481,9 @@ pub fn get_local_toolchain(
     let lib_dir = get_lib_dir_from_rustc(&rustc).context("Cannot find libdir for rustc")?;
 
     Ok(Toolchain {
-        components: ToolchainComponents::from_binaries_and_libdir(rustc, rustdoc, cargo, &lib_dir)?,
+        components: ToolchainComponents::from_binaries_and_libdir(
+            rustc, rustdoc, clippy, cargo, &lib_dir,
+        )?,
         id,
         triple: target_triple,
     })
@@ -465,16 +520,23 @@ pub fn create_toolchain_from_published_version(
     };
     let rustc = which("rustc")?;
     let rustdoc = which("rustdoc")?;
+    let clippy = which("clippy")?;
     let cargo = which("cargo")?;
 
     debug!("Found rustc: {}", rustc.display());
     debug!("Found rustdoc: {}", rustdoc.display());
+    debug!("Found clippy: {}", clippy.display());
     debug!("Found cargo: {}", cargo.display());
 
     let lib_dir = get_lib_dir_from_rustc(&rustc)?;
 
-    let components =
-        ToolchainComponents::from_binaries_and_libdir(rustc, Some(rustdoc), cargo, &lib_dir)?;
+    let components = ToolchainComponents::from_binaries_and_libdir(
+        rustc,
+        Some(rustdoc),
+        Some(clippy),
+        cargo,
+        &lib_dir,
+    )?;
 
     Ok(Toolchain {
         components,

database/src/lib.rs

@@ -224,6 +224,8 @@ pub enum Profile {
     Doc,
     /// An optimized "release" build
     Opt,
+    /// A Clippy run
+    Clippy,
 }
 
 impl Profile {
@@ -233,6 +235,7 @@ impl Profile {
             Profile::Opt => "opt",
             Profile::Debug => "debug",
             Profile::Doc => "doc",
+            Profile::Clippy => "clippy",
         }
     }
 }
@@ -245,6 +248,7 @@ impl std::str::FromStr for Profile {
             "debug" => Profile::Debug,
             "doc" => Profile::Doc,
             "opt" => Profile::Opt,
+            "clippy" => Profile::Clippy,
             _ => return Err(format!("{} is not a profile", s)),
         })
     }