make better

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)
+                    // Cargo always exits with 101 when rustc has an error.
-                        || output.contains("internal compiler error"),
+                    output.contains("internal compiler error") || output.contains("' panicked at"),
                     output,
                 )
             }

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

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

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)
+        fs::write(self.path, new).with_context(|| format!("writing file {}", self.path.display()))
-            .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

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<SourceFile>,
     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<Item = Box<dyn Processor + 'a>>,
     ) -> 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<String>);
+
 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<AstPath> },
+    InitialCollection {
-    Bisecting { current: HashSet<AstPath>, worklist: Vec<Vec<AstPath>> },
+        candidates: Vec<AstPath>,
+    },
+    Bisecting {
+        committed: BTreeSet<AstPath>,
+        failed: BTreeSet<AstPath>,
+        current: BTreeSet<AstPath>,
+        worklist: Vec<Vec<AstPath>>,
+    },
     Success,
 }
+
+fn split_owned<T, From: IntoIterator<Item = T>, A: FromIterator<T>, B: FromIterator<T>>(
+    vec: From,
+) -> (A, B) {
+    let candidates = vec.into_iter().collect::<Vec<_>>();
+    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, .. } => {
+            PassControllerState::Bisecting {
-                match worklist.pop() {
+                committed,
-                    Some(next) => *current = next.into_iter().collect(),
+                failed: _,
-                    None => {
+                current,
-                        self.state = PassControllerState::Success;
+                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 (current, first_worklist_item) = split_owned(mem::take(candidates));
-                let half = candidates.len() / 2;
+
-                let (first_half, second_half) = candidates.split_at(half);
+                self.state = PassControllerState::Bisecting {
-                self
+                    committed: BTreeSet::new(),
-                    .state = PassControllerState::Bisecting {
+                    failed: BTreeSet::new(),
-                    current: first_half.iter().cloned().collect(),
+                    current,
-                    worklist: vec![second_half.to_owned()],
+                    worklist: vec![first_worklist_item],
                 };
             }
-            PassControllerState::Bisecting { current, worklist } => {
+            PassControllerState::Bisecting {
-                dbg!(& current, & worklist);
+                committed,
-                todo!();
+                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!(
+                unreachable!("No change while bisecting, current was empty somehow: {current:?}");
-                    "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_fn_mut);
-        tracking!(visit_item_impl_mut); tracking!(visit_item_mod_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
+        fn visit_item_fn_mut(&mut self, func: &mut syn::ItemFn) {
-        .push(func.sig.ident.to_string()); syn::visit_mut::visit_item_fn_mut(self, func);
+            self.current_path.push(func.sig.ident.to_string());
-        self.current_path.pop(); }
+            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) {
+        fn visit_impl_item_method_mut(&mut self, method: &mut syn::ImplItemMethod) {
-        self.current_path.push(method.sig.ident.to_string());
+            self.current_path.push(method.sig.ident.to_string());
-        syn::visit_mut::visit_impl_item_method_mut(self, method); self.current_path
+            syn::visit_mut::visit_impl_item_method_mut(self, method);
-        .pop(); }
+            self.current_path.pop();
+        }
     };
     (visit_item_impl_mut) => {
-        fn visit_item_impl_mut(& mut self, item : & mut syn::ItemImpl) { self
+        fn visit_item_impl_mut(&mut self, item: &mut syn::ItemImpl) {
-        .current_path.push(item.self_ty.clone().into_token_stream().to_string());
+            self.current_path
-        syn::visit_mut::visit_item_impl_mut(self, item); self.current_path.pop(); }
+                .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
+        fn visit_item_mod_mut(&mut self, module: &mut syn::ItemMod) {
-        .current_path.push(module.ident.to_string());
+            self.current_path.push(module.ident.to_string());
-        syn::visit_mut::visit_item_mod_mut(self, module); self.current_path.pop(); }
+            syn::visit_mut::visit_item_mod_mut(self, module);
+            self.current_path.pop();
+        }
     };
 }
 pub(crate) use tracking;

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

test-cases/unused-code/src/main.rs

@@ -4,5 +4,4 @@ fn unused() {
 fn main() {
     other::unused();
 }
-
 mod other;