From dcd163aeda5f5f9d60fa04da29ebd11a02578b79 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sat, 17 Dec 2022 22:39:05 +0100 Subject: [PATCH] kinda works --- src/build.rs | 31 +++++++------ src/everybody_loops.rs | 3 +- src/lib.rs | 4 +- src/privatize.rs | 4 +- src/processor/files.rs | 5 +-- src/processor/mod.rs | 54 +++++++++++++++-------- src/processor/reaper.rs | 67 +++++++++++++++++++++-------- test-cases/unused-code/src/other.rs | 2 +- 8 files changed, 111 insertions(+), 59 deletions(-) diff --git a/src/build.rs b/src/build.rs index e74549b..3099758 100644 --- a/src/build.rs +++ b/src/build.rs @@ -1,12 +1,17 @@ use anyhow::{Context, Result}; use rustfix::diagnostics::Diagnostic; use serde::Deserialize; -use std::{collections::HashSet, fmt::Display, path::PathBuf, process::Command}; +use std::{collections::HashSet, fmt::Display, path::PathBuf, process::Command, rc::Rc}; use crate::Options; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Build { + inner: Rc, +} + +#[derive(Debug)] +struct BuildInner { mode: BuildMode, input_path: PathBuf, no_verify: bool, @@ -29,21 +34,23 @@ impl Build { BuildMode::Rustc }; Self { - mode, - input_path: options.path.clone(), - no_verify: options.no_verify, + inner: Rc::new(BuildInner { + mode, + input_path: options.path.clone(), + no_verify: options.no_verify, + }), } } pub fn build(&self) -> Result { - if self.no_verify { + if self.inner.no_verify { return Ok(BuildResult { reproduces_issue: false, no_verify: true, }); } - let reproduces_issue = match &self.mode { + let reproduces_issue = match &self.inner.mode { BuildMode::Cargo => { let mut cmd = Command::new("cargo"); cmd.arg("build"); @@ -62,7 +69,7 @@ impl Build { BuildMode::Rustc => { let mut cmd = Command::new("rustc"); cmd.args(["--edition", "2018"]); - cmd.arg(&self.input_path); + cmd.arg(&self.inner.input_path); cmd.output() .context("spawning rustc process")? @@ -74,12 +81,12 @@ impl Build { Ok(BuildResult { reproduces_issue, - no_verify: self.no_verify, + no_verify: self.inner.no_verify, }) } - pub fn get_suggestions(&self) -> Result<(Vec, Vec)> { - let diags = match self.mode { + pub fn get_diags(&self) -> Result<(Vec, Vec)> { + let diags = match self.inner.mode { BuildMode::Cargo => { let mut cmd = Command::new("cargo"); cmd.args(["build", "--message-format=json"]); @@ -102,7 +109,7 @@ impl Build { BuildMode::Rustc => { let mut cmd = std::process::Command::new("rustc"); cmd.args(["--edition", "2018", "--error-format=json"]); - cmd.arg(&self.input_path); + cmd.arg(&self.inner.input_path); let output = cmd.output()?.stderr; let output = String::from_utf8(output)?; diff --git a/src/everybody_loops.rs b/src/everybody_loops.rs index 2952fe6..02f1ab6 100644 --- a/src/everybody_loops.rs +++ b/src/everybody_loops.rs @@ -1,7 +1,7 @@ use quote::ToTokens; use syn::{parse_quote, visit_mut::VisitMut}; -use crate::processor::{ProcessChecker, ProcessState, Processor}; +use crate::processor::{ProcessChecker, ProcessState, Processor, SourceFile}; struct Visitor<'a> { current_path: Vec, @@ -68,6 +68,7 @@ impl Processor for EverybodyLoops { fn process_file( &mut self, krate: &mut syn::File, + _: &SourceFile, checker: &mut ProcessChecker, ) -> ProcessState { let mut visitor = Visitor::new(checker); diff --git a/src/lib.rs b/src/lib.rs index 71d6429..113182d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,7 +12,7 @@ use anyhow::{Context, Result}; use clap::Parser; use processor::Minimizer; -use crate::{everybody_loops::EverybodyLoops, processor::Processor, privatize::Privatize}; +use crate::{everybody_loops::EverybodyLoops, privatize::Privatize, processor::Processor}; #[derive(clap::Parser)] pub struct Options { @@ -30,7 +30,7 @@ pub fn minimize() -> Result<()> { let build = build::Build::new(&options); - let mut minimizer = Minimizer::new_glob_dir(&options.path, build, &options); + let mut minimizer = Minimizer::new_glob_dir(&options.path, build); println!("{minimizer:?}"); diff --git a/src/privatize.rs b/src/privatize.rs index 11dd03c..88f0019 100644 --- a/src/privatize.rs +++ b/src/privatize.rs @@ -1,6 +1,6 @@ use syn::{parse_quote, visit_mut::VisitMut, Visibility}; -use crate::processor::{ProcessChecker, Processor, ProcessState}; +use crate::processor::{ProcessChecker, ProcessState, Processor, SourceFile}; struct Visitor { pub_crate: Visibility, @@ -29,7 +29,7 @@ impl VisitMut for Visitor { pub struct Privatize {} impl Processor for Privatize { - fn process_file(&mut self, krate: &mut syn::File, _: &mut ProcessChecker) -> ProcessState { + fn process_file(&mut self, krate: &mut syn::File, _: &SourceFile, _: &mut ProcessChecker) -> ProcessState { let mut visitor = Visitor::new(); visitor.visit_file_mut(krate); visitor.process_state diff --git a/src/processor/files.rs b/src/processor/files.rs index 7a64bf8..32f39ca 100644 --- a/src/processor/files.rs +++ b/src/processor/files.rs @@ -4,7 +4,7 @@ use std::{ path::{Path, PathBuf}, }; -#[derive(Debug, PartialEq, Eq, Hash)] +#[derive(Debug, PartialEq, Eq, Clone, Hash)] pub struct SourceFile { pub path: PathBuf, } @@ -72,9 +72,8 @@ impl SourceFile { } } - impl Changes { pub fn had_changes(&self) -> bool { self.any_change } -} \ No newline at end of file +} diff --git a/src/processor/mod.rs b/src/processor/mod.rs index c1b37da..50eb686 100644 --- a/src/processor/mod.rs +++ b/src/processor/mod.rs @@ -5,13 +5,24 @@ use std::{collections::HashSet, ffi::OsStr, path::Path}; use anyhow::{ensure, Context, Result}; -use crate::{build::Build, processor::files::Changes, Options}; +use crate::{build::Build, processor::files::Changes}; -use self::files::SourceFile; +pub use self::files::SourceFile; pub trait Processor { - fn process_file(&mut self, krate: &mut syn::File, checker: &mut ProcessChecker) - -> ProcessState; + fn refresh_state(&mut self) -> Result<()> { + Ok(()) + } + + /// Process a file. The state of the processor might get invalidated in the process as signaled with + /// `ProcessState::FileInvalidated`. When a file is invalidated, the minimizer will call `Processor::refersh_state` + /// before calling the this function on the same file again. + fn process_file( + &mut self, + krate: &mut syn::File, + file: &SourceFile, + checker: &mut ProcessChecker, + ) -> ProcessState; fn name(&self) -> &'static str; } @@ -27,11 +38,10 @@ pub enum ProcessState { pub struct Minimizer { files: Vec, build: Build, - no_verify: bool, } impl Minimizer { - pub fn new_glob_dir(path: &Path, build: Build, options: &Options) -> Self { + pub fn new_glob_dir(path: &Path, build: Build) -> Self { let walk = walkdir::WalkDir::new(path); let files = walk @@ -49,29 +59,25 @@ impl Minimizer { }) .collect(); - Self { - files, - build, - no_verify: options.no_verify, - } + Self { files, build } } pub fn run_passes<'a>( &mut self, - passes: impl IntoIterator>, + passes: impl IntoIterator>, ) -> Result<()> { let inital_build = self.build.build()?; println!("Initial build: {}", inital_build); - if !self.no_verify { - ensure!( - inital_build.reproduces_issue(), - "Initial build must reproduce issue" - ); - } + ensure!( + inital_build.reproduces_issue(), + "Initial build must reproduce issue" + ); let mut invalidated_files = HashSet::new(); for mut pass in passes { + let mut refresh_and_try_again = false; + 'pass: loop { println!("Starting a round of {}", pass.name()); let mut changes = Changes::default(); @@ -88,7 +94,7 @@ impl Minimizer { let mut krate = syn::parse_file(change.before_content()) .with_context(|| format!("parsing file {file_display}"))?; - let has_made_change = pass.process_file(&mut krate, &mut ProcessChecker {}); + let has_made_change = pass.process_file(&mut krate, file, &mut ProcessChecker {}); match has_made_change { ProcessState::Changed | ProcessState::FileInvalidated => { @@ -117,8 +123,18 @@ impl Minimizer { } if !changes.had_changes() { + if !refresh_and_try_again && invalidated_files.len() > 0 { + // A few files have been invalidated, let's refresh and try these again. + pass.refresh_state().context("refreshing state for pass")?; + refresh_and_try_again = true; + println!("Refreshing files for {}", pass.name()); + continue; + } + println!("Finished {}", pass.name()); break 'pass; + } else { + refresh_and_try_again = false; } } } diff --git a/src/processor/reaper.rs b/src/processor/reaper.rs index 8bf4d56..4b1b5dc 100644 --- a/src/processor/reaper.rs +++ b/src/processor/reaper.rs @@ -1,10 +1,16 @@ //! Deletes dead code. -use super::{files::Changes, Minimizer, ProcessState, Processor}; +use crate::build::Build; + +use super::{files::Changes, Minimizer, ProcessState, Processor, SourceFile}; use anyhow::{ensure, Context, Result}; use proc_macro2::Span; use rustfix::{diagnostics::Diagnostic, Suggestion}; -use std::{collections::HashMap, ops::Range, path::Path}; +use std::{ + collections::{HashMap, HashSet}, + ops::Range, + path::Path, +}; use syn::{visit_mut::VisitMut, ImplItem, Item}; fn file_for_suggestion(suggestion: &Suggestion) -> &str { @@ -15,16 +21,14 @@ impl Minimizer { pub fn delete_dead_code(&mut self) -> Result<()> { let inital_build = self.build.build()?; println!("Before reaper: {}", inital_build); - if !self.no_verify { - ensure!( - inital_build.reproduces_issue(), - "Initial build must reproduce issue" - ); - } + ensure!( + inital_build.reproduces_issue(), + "Initial build must reproduce issue" + ); let (diags, suggestions) = self .build - .get_suggestions() + .get_diags() .context("getting suggestions from rustc")?; let mut suggestions_for_file = HashMap::<_, Vec<_>>::new(); @@ -38,11 +42,10 @@ impl Minimizer { // Always unconditionally apply unused imports. self.apply_unused_imports(&suggestions_for_file)?; - self.run_passes([Box::new(DeleteUnusedFunctions { - diags, - invalid: false, - }) as Box]) - .context("deleting unused functions")?; + self.run_passes([ + Box::new(DeleteUnusedFunctions::new(self.build.clone(), diags)) as Box, + ]) + .context("deleting unused functions")?; Ok(()) } @@ -91,22 +94,44 @@ impl Minimizer { struct DeleteUnusedFunctions { diags: Vec, - invalid: bool, + build: Build, + invalid: HashSet, +} + +impl DeleteUnusedFunctions { + fn new(build: Build, diags: Vec) -> Self { + DeleteUnusedFunctions { + diags, + build, + invalid: HashSet::new(), + } + } } impl Processor for DeleteUnusedFunctions { + fn refresh_state(&mut self) -> Result<()> { + let (diags, _) = self.build.get_diags().context("getting diagnostics")?; + self.diags = diags; + self.invalid = HashSet::new(); + Ok(()) + } + fn process_file( &mut self, krate: &mut syn::File, + file: &SourceFile, _: &mut super::ProcessChecker, ) -> ProcessState { - assert!(!self.invalid, "processing with invalid state"); + assert!( + !self.invalid.contains(file), + "processing with invalid state" + ); - let mut visitor = FindUnusedFunction::new(self.diags.iter()); + let mut visitor = FindUnusedFunction::new(file, self.diags.iter()); visitor.visit_file_mut(krate); if visitor.process_state == ProcessState::FileInvalidated { - self.invalid = true; + self.invalid.insert(file.clone()); } visitor.process_state @@ -143,7 +168,7 @@ struct FindUnusedFunction { } impl FindUnusedFunction { - fn new<'a>(diags: impl Iterator) -> Self { + fn new<'a>(file: &SourceFile, diags: impl Iterator) -> Self { let unused_functions = diags .filter_map(|diag| { // FIXME: use `code` correctly @@ -167,6 +192,10 @@ impl FindUnusedFunction { "encountered multiline span in dead_code" ); + if Path::new(&span.file_name) != file.path { + return None; + } + Some(Unused { name, line: span.line_start, diff --git a/test-cases/unused-code/src/other.rs b/test-cases/unused-code/src/other.rs index e03a252..1dcbcc3 100644 --- a/test-cases/unused-code/src/other.rs +++ b/test-cases/unused-code/src/other.rs @@ -1,3 +1,3 @@ fn unused() { loop {} -} \ No newline at end of file +}