diff --git a/src/build.rs b/src/build.rs index 6c40706..15e4873 100644 --- a/src/build.rs +++ b/src/build.rs @@ -92,7 +92,7 @@ impl Build { let (is_ice, output) = match &self.inner.mode { BuildMode::Cargo { args } => { let mut cmd = Command::new("cargo"); - cmd.arg("build"); + cmd.args(["build", "--color=always"]); for arg in args.iter().flatten() { cmd.arg(arg); @@ -107,8 +107,8 @@ impl Build { let output = String::from_utf8(outputs.stderr)?; ( - outputs.status.code() == Some(101) - || output.contains("internal compiler error"), + // Cargo always exits with 101 when rustc has an error. + output.contains("internal compiler error") || output.contains("' panicked at"), output, ) } diff --git a/src/lib.rs b/src/lib.rs index 1128b3f..3c04294 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,26 +83,5 @@ pub fn minimize() -> Result<()> { minimizer.delete_dead_code().context("deleting dead code")?; - /* - let file = expand::expand(&dir).context("during expansion")?; - - //let file = syn::parse_str("extern { pub fn printf(format: *const ::c_char, ...) -> ::c_int; }",).unwrap(); - let file = prettyplease::unparse(&file); - - println!("// EXPANDED-START\n\n{file}\n\n// EXPANDED-END"); - - std::fs::write("expanded.rs", file)?; - - println!("wow, expanded"); - */ - - /* - let build = Build::new(cargo_dir); - - if build.build()?.success { - bail!("build must initially fail!"); - } - */ - Ok(()) } diff --git a/src/main.rs b/src/main.rs index 0ea65b9..9811520 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,9 +1,14 @@ use anyhow::Result; -use tracing::info; +use tracing::{error, info, Level}; use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, EnvFilter, Registry}; fn main() -> Result<()> { - let registry = Registry::default().with(EnvFilter::from_default_env()); + let registry = Registry::default().with( + EnvFilter::builder() + .with_default_directive(Level::INFO.into()) + .from_env() + .unwrap(), + ); info!("Starting cargo-minimize"); @@ -13,7 +18,9 @@ fn main() -> Result<()> { registry.with(tree_layer).init(); - cargo_minimize::minimize()?; + if let Err(err) = cargo_minimize::minimize() { + error!("An error occured:\n{err}"); + } Ok(()) } diff --git a/src/processor/files.rs b/src/processor/files.rs index 3e1fae2..407cd23 100644 --- a/src/processor/files.rs +++ b/src/processor/files.rs @@ -1,40 +1,50 @@ use anyhow::{Context, Result}; -use std::{fs, path::{Path, PathBuf}}; +use std::{ + fs, + path::{Path, PathBuf}, +}; + #[derive(Debug, PartialEq, Eq, Clone, Hash)] pub(crate) struct SourceFile { pub(crate) path: PathBuf, } + #[derive(Default)] pub(crate) struct Changes { any_change: bool, } + pub(crate) struct FileChange<'a, 'b> { pub(crate) path: &'a Path, content: String, changes: &'b mut Changes, has_written_change: bool, } + impl FileChange<'_, '_> { pub(crate) fn before_content(&self) -> &str { &self.content } + pub(crate) fn write(&mut self, new: &str) -> Result<()> { self.has_written_change = true; - fs::write(self.path, new) - .with_context(|| format!("writing file {}", self.path.display())) + fs::write(self.path, new).with_context(|| format!("writing file {}", self.path.display())) } + pub(crate) fn rollback(mut self) -> Result<()> { assert!(self.has_written_change); self.has_written_change = false; fs::write(self.path, &self.content) .with_context(|| format!("writing file {}", self.path.display())) } + pub(crate) fn commit(mut self) { assert!(self.has_written_change); self.has_written_change = false; self.changes.any_change = true; } } + impl Drop for FileChange<'_, '_> { fn drop(&mut self) { if self.has_written_change { @@ -45,6 +55,7 @@ impl Drop for FileChange<'_, '_> { } } } + impl SourceFile { pub(crate) fn try_change<'file, 'change>( &'file self, @@ -60,6 +71,7 @@ impl SourceFile { }) } } + impl Changes { pub(crate) fn had_changes(&self) -> bool { self.any_change diff --git a/src/processor/mod.rs b/src/processor/mod.rs index b611bc0..02b867f 100644 --- a/src/processor/mod.rs +++ b/src/processor/mod.rs @@ -1,13 +1,22 @@ mod files; mod reaper; -use std::{borrow::Borrow, collections::HashSet, ffi::OsStr, fmt::Debug, mem, path::Path}; -use anyhow::{Context, Result}; -use crate::{build::Build, processor::files::Changes}; pub(crate) use self::files::SourceFile; +use crate::{build::Build, processor::files::Changes}; +use anyhow::{Context, Result}; +use std::{ + borrow::Borrow, + collections::{BTreeSet, HashSet}, + ffi::OsStr, + fmt::Debug, + mem, + path::Path, +}; + pub(crate) trait Processor { 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. @@ -17,24 +26,29 @@ pub(crate) trait Processor { file: &SourceFile, checker: &mut PassController, ) -> ProcessState; + fn name(&self) -> &'static str; } + impl Debug for dyn Processor { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(self.name()) } } + #[derive(Debug, PartialEq, Eq)] pub(crate) enum ProcessState { NoChange, Changed, FileInvalidated, } + #[derive(Debug)] pub(crate) struct Minimizer { files: Vec, build: Build, } + impl Minimizer { pub(crate) fn new_glob_dir(path: &Path, build: Build) -> Self { let walk = walkdir::WalkDir::new(path); @@ -43,7 +57,7 @@ impl Minimizer { .filter_map(|entry| match entry { Ok(entry) => Some(entry), Err(err) => { - eprintln!("WARN: Error in walkdir: {err}"); + warn!("Error during walkdir: {err}"); None } }) @@ -52,23 +66,28 @@ impl Minimizer { path: entry.into_path(), }) .inspect(|file| { - println!("- {}", file.path.display()); + info!("Collecting file: {}", file.path.display()); }) .collect(); + Self { files, build } } + pub(crate) fn run_passes<'a>( &self, passes: impl IntoIterator>, ) -> Result<()> { let inital_build = self.build.build()?; - println!("Initial build: {inital_build}"); + info!("Initial build: {inital_build}"); inital_build.require_reproduction("Initial")?; + for mut pass in passes { self.run_pass(&mut *pass)?; } + Ok(()) } + fn run_pass(&self, pass: &mut dyn Processor) -> Result<()> { let mut invalidated_files = HashSet::new(); let mut refresh_and_try_again = false; @@ -76,27 +95,32 @@ impl Minimizer { let span = info_span!("Starting round of pass", name = pass.name()); let _enter = span.enter(); let mut changes = Changes::default(); + for file in &self.files { if invalidated_files.contains(file) { continue; } self.process_file(pass, file, &mut invalidated_files, &mut changes)?; } + if !changes.had_changes() { if !refresh_and_try_again && !invalidated_files.is_empty() { pass.refresh_state().context("refreshing state for pass")?; invalidated_files.clear(); refresh_and_try_again = true; - println!("Refreshing files for {}", pass.name()); + info!("Refreshing files for {}", pass.name()); continue; } - println!("Finished {}", pass.name()); + + info!("Finished {}", pass.name()); + return Ok(()); } else { refresh_and_try_again = false; } } } + fn process_file<'file>( &self, pass: &mut dyn Processor, @@ -106,18 +130,19 @@ impl Minimizer { ) -> Result<()> { let mut checker = PassController::new(); loop { - dbg!(& checker); + debug!(?checker); let file_display = file.path.display(); let mut change = file.try_change(changes)?; 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, file, &mut checker); + match has_made_change { ProcessState::Changed | ProcessState::FileInvalidated => { let result = prettyplease::unparse(&krate); change.write(&result)?; let after = self.build.build()?; - println!("{file_display}: After {}: {after}", pass.name()); + info!("{file_display}: After {}: {after}", pass.name()); if after.reproduces_issue() { change.commit(); checker.reproduces(); @@ -130,10 +155,11 @@ impl Minimizer { } } ProcessState::NoChange => { - println!("{file_display}: After {}: no changes", pass.name()); + info!("{file_display}: After {}: no changes", pass.name()); checker.no_change(); } } + if checker.is_finished() { break; } @@ -141,28 +167,55 @@ impl Minimizer { Ok(()) } } -#[derive(Clone, PartialEq, Eq, Hash)] + +#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] struct AstPath(Vec); + impl Borrow<[String]> for AstPath { fn borrow(&self) -> &[String] { &self.0 } } + impl Debug for AstPath { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "AstPath({:?})", self.0) } } + #[derive(Debug)] pub(crate) struct PassController { state: PassControllerState, } + #[derive(Debug)] enum PassControllerState { - InitialCollection { candidates: Vec }, - Bisecting { current: HashSet, worklist: Vec> }, + InitialCollection { + candidates: Vec, + }, + Bisecting { + committed: BTreeSet, + failed: BTreeSet, + current: BTreeSet, + worklist: Vec>, + }, Success, } + +fn split_owned, A: FromIterator, B: FromIterator>( + vec: From, +) -> (A, B) { + let candidates = vec.into_iter().collect::>(); + let half = candidates.len() / 2; + + let mut candidates = candidates.into_iter(); + + let first_half = candidates.by_ref().take(half).collect(); + let second_half = candidates.collect(); + + (first_half, second_half) +} + impl PassController { fn new() -> Self { Self { @@ -171,41 +224,80 @@ impl PassController { }, } } + + fn next_in_worklist(&mut self) { + match &mut self.state { + PassControllerState::Bisecting { + current, worklist, .. + } => match worklist.pop() { + Some(next) => { + *current = next.into_iter().collect(); + } + None => { + self.state = PassControllerState::Success; + } + }, + _ => unreachable!("next_in_worklist called on non-bisecting state"), + } + } + fn reproduces(&mut self) { match &mut self.state { PassControllerState::InitialCollection { .. } => { self.state = PassControllerState::Success; } - PassControllerState::Bisecting { current, worklist, .. } => { - match worklist.pop() { - Some(next) => *current = next.into_iter().collect(), - None => { - self.state = PassControllerState::Success; - } - } + PassControllerState::Bisecting { + committed, + failed: _, + current, + worklist: _, + } => { + committed.extend(mem::take(current)); + + self.next_in_worklist(); } PassControllerState::Success => unreachable!("Processed after success"), } } + fn does_not_reproduce(&mut self) { match &mut self.state { PassControllerState::InitialCollection { candidates } => { - let candidates = mem::take(candidates); - let half = candidates.len() / 2; - let (first_half, second_half) = candidates.split_at(half); - self - .state = PassControllerState::Bisecting { - current: first_half.iter().cloned().collect(), - worklist: vec![second_half.to_owned()], + let (current, first_worklist_item) = split_owned(mem::take(candidates)); + + self.state = PassControllerState::Bisecting { + committed: BTreeSet::new(), + failed: BTreeSet::new(), + current, + worklist: vec![first_worklist_item], }; } - PassControllerState::Bisecting { current, worklist } => { - dbg!(& current, & worklist); - todo!(); + PassControllerState::Bisecting { + committed, + failed, + current, + worklist, + } => { + debug!(?committed, ?current, ?worklist); + + if current.len() == 1 { + // We are at a leaf. This is a failure. + // FIXME: We should retry the failed ones until a fixpoint is reached. + failed.extend(mem::take(current)); + } else { + // Split it further and add it to the worklist. + let (first_half, second_half) = split_owned(mem::take(current)); + + worklist.push(first_half); + worklist.push(second_half); + } + + self.next_in_worklist() } PassControllerState::Success => unreachable!("Processed after success"), } } + fn no_change(&mut self) { match &self.state { PassControllerState::InitialCollection { candidates } => { @@ -216,13 +308,12 @@ impl PassController { self.state = PassControllerState::Success; } PassControllerState::Bisecting { current, .. } => { - unreachable!( - "No change while bisecting, current was empty somehow: {current:?}" - ); + unreachable!("No change while bisecting, current was empty somehow: {current:?}"); } PassControllerState::Success => {} } } + fn is_finished(&mut self) -> bool { match &mut self.state { PassControllerState::InitialCollection { .. } => false, @@ -230,6 +321,7 @@ impl PassController { PassControllerState::Success => true, } } + pub(crate) fn can_process(&mut self, path: &[String]) -> bool { match &mut self.state { PassControllerState::InitialCollection { candidates } => { @@ -243,31 +335,42 @@ impl PassController { } } } + macro_rules! tracking { () => { - tracking!(visit_item_fn_mut); tracking!(visit_impl_item_method_mut); - tracking!(visit_item_impl_mut); tracking!(visit_item_mod_mut); + tracking!(visit_item_fn_mut); + tracking!(visit_impl_item_method_mut); + tracking!(visit_item_impl_mut); + tracking!(visit_item_mod_mut); }; (visit_item_fn_mut) => { - fn visit_item_fn_mut(& mut self, func : & mut syn::ItemFn) { self.current_path - .push(func.sig.ident.to_string()); syn::visit_mut::visit_item_fn_mut(self, func); - self.current_path.pop(); } + fn visit_item_fn_mut(&mut self, func: &mut syn::ItemFn) { + self.current_path.push(func.sig.ident.to_string()); + syn::visit_mut::visit_item_fn_mut(self, func); + self.current_path.pop(); + } }; (visit_impl_item_method_mut) => { - fn visit_impl_item_method_mut(& mut self, method : & mut syn::ImplItemMethod) { - self.current_path.push(method.sig.ident.to_string()); - syn::visit_mut::visit_impl_item_method_mut(self, method); self.current_path - .pop(); } + fn visit_impl_item_method_mut(&mut self, method: &mut syn::ImplItemMethod) { + self.current_path.push(method.sig.ident.to_string()); + syn::visit_mut::visit_impl_item_method_mut(self, method); + self.current_path.pop(); + } }; (visit_item_impl_mut) => { - fn visit_item_impl_mut(& mut self, item : & mut syn::ItemImpl) { self - .current_path.push(item.self_ty.clone().into_token_stream().to_string()); - syn::visit_mut::visit_item_impl_mut(self, item); self.current_path.pop(); } + fn visit_item_impl_mut(&mut self, item: &mut syn::ItemImpl) { + self.current_path + .push(item.self_ty.clone().into_token_stream().to_string()); + syn::visit_mut::visit_item_impl_mut(self, item); + self.current_path.pop(); + } }; (visit_item_mod_mut) => { - fn visit_item_mod_mut(& mut self, module : & mut syn::ItemMod) { self - .current_path.push(module.ident.to_string()); - syn::visit_mut::visit_item_mod_mut(self, module); self.current_path.pop(); } + fn visit_item_mod_mut(&mut self, module: &mut syn::ItemMod) { + self.current_path.push(module.ident.to_string()); + syn::visit_mut::visit_item_mod_mut(self, module); + self.current_path.pop(); + } }; } pub(crate) use tracking; diff --git a/src/processor/reaper.rs b/src/processor/reaper.rs index efc1aa9..f9ca4ec 100644 --- a/src/processor/reaper.rs +++ b/src/processor/reaper.rs @@ -23,7 +23,7 @@ fn file_for_suggestion(suggestion: &Suggestion) -> &str { impl Minimizer { pub fn delete_dead_code(&mut self) -> Result<()> { let inital_build = self.build.build()?; - println!("Before reaper: {inital_build}"); + info!("Before reaper: {inital_build}"); inital_build.require_reproduction("Initial")?; @@ -80,7 +80,7 @@ impl Minimizer { let after = self.build.build()?; - println!("{}: After reaper: {after}", file.path.display()); + info!("{}: After reaper: {after}", file.path.display()); if after.reproduces_issue() { change.commit(); diff --git a/test-cases/unused-code/src/main.rs b/test-cases/unused-code/src/main.rs index 46fe8c7..3c75dd7 100644 --- a/test-cases/unused-code/src/main.rs +++ b/test-cases/unused-code/src/main.rs @@ -4,5 +4,4 @@ fn unused() { fn main() { other::unused(); } - mod other;