Touch up controller logic to handle nested items well.

The test in the previous commit now passes

full-tests/nested-items2.rs

@@ -0,0 +1,24 @@
+// this should all get deleted in a single swoop *and* not panic about it
+/// ~REQUIRE-DELETED l1
+mod l1 {
+    mod l2 {
+        mod l3 {
+            mod l4{
+                mod l5 {
+                    fn foo(){}
+                    fn bar(){}
+                    mod l6 {
+                        fn x1(){}
+                    }
+                    fn x2(){}
+                }
+            }
+            mod l4_2 {
+                fn y(){}
+            }
+        }
+    }
+    fn x8(){}
+}
+/// ~MINIMIZE-ROOT main
+fn main(){}

src/processor/checker.rs

@@ -6,6 +6,14 @@ use self::worklist::Worklist;
 
 #[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 struct AstPath(Vec<String>);
+impl AstPath {
+    fn has_prefix(&self, other: &AstPath) -> bool {
+        if self.0.len() < other.0.len() {
+            return false;
+        }
+        std::iter::zip(self.0.iter(), other.0.iter()).all(|(a, b)| a == b)
+    }
+}
 
 impl Borrow<[String]> for AstPath {
     fn borrow(&self) -> &[String] {
@@ -75,6 +83,20 @@ mod worklist {
         pub(super) fn pop(&mut self) -> Option<Vec<AstPath>> {
             self.0.pop()
         }
+
+        // remove all the worklist items that would have been covered by the
+        // given ones.
+        // I.e. if we have already deleted the entire module, there's no need
+        // trying to delete that module's individual items anymore
+        pub(super) fn prune(&mut self, things: &std::collections::BTreeSet<AstPath>) {
+            for wl in &mut self.0 {
+                wl.retain(|path| {
+                    // retain only if none of the things are a prefix of this path
+                    things.iter().all(|thing| !path.has_prefix(thing))
+                })
+            }
+            self.0.retain(|wl| !wl.is_empty());
+        }
     }
 }
 
@@ -97,8 +119,9 @@ impl PassController {
                 committed,
                 failed: _,
                 current,
-                worklist: _,
+                worklist,
             } => {
+                worklist.prune(current);
                 committed.extend(mem::take(current));
 
                 self.next_in_worklist();
@@ -110,19 +133,8 @@ impl PassController {
     /// The changes did not reproduce the regression. Bisect further.
     pub fn does_not_reproduce(&mut self) {
         match &mut self.state {
-            PassControllerState::InitialCollection { candidates } => {
+            PassControllerState::InitialCollection { candidates: _ } => {
-                // Applying them all was too much, let's bisect!
+                unreachable!("we should have made no changes on initial collection, what do you mean it does not reproduce?!?")
-                let (current, first_worklist_item) = split_owned(mem::take(candidates));
-
-                let mut worklist = Worklist::new();
-                worklist.push(first_worklist_item);
-
-                self.state = PassControllerState::Bisecting {
-                    committed: BTreeSet::new(),
-                    failed: BTreeSet::new(),
-                    current,
-                    worklist,
-                };
             }
             PassControllerState::Bisecting {
                 committed,
@@ -155,15 +167,24 @@ impl PassController {
         }
     }
 
-    /// The pass did not apply any changes. We're done.
+    /// The pass did not apply any changes. We're either done or just starting
     pub fn no_change(&mut self) {
-        match &self.state {
+        match &mut self.state {
             PassControllerState::InitialCollection { candidates } => {
-                assert!(
+                if candidates.is_empty() {
-                    candidates.is_empty(),
+                    self.state = PassControllerState::Success;
-                    "No change but received candidates. The responsible pass does not seem to track the ProcessState correctly: {candidates:?}"
+                } else {
-                );
+                    let current = mem::take(candidates)
-                self.state = PassControllerState::Success;
+                        .into_iter()
+                        .collect::<BTreeSet<AstPath>>();
+
+                    self.state = PassControllerState::Bisecting {
+                        committed: BTreeSet::new(),
+                        failed: BTreeSet::new(),
+                        current,
+                        worklist: Worklist::new(),
+                    };
+                }
             }
             PassControllerState::Bisecting { current, .. } => {
                 unreachable!("Pass said it didn't change anything in the bisection phase, nils forgot what this means: {current:?}");
@@ -184,9 +205,9 @@ impl PassController {
     pub fn can_process(&mut self, path: &[String]) -> bool {
         match &mut self.state {
             PassControllerState::InitialCollection { candidates } => {
-                // For the initial collection, we collect the candidate and apply them all.
+                // For the initial collection, we collect the candidate but don't apply anything
                 candidates.push(AstPath(path.to_owned()));
-                true
+                false
             }
             PassControllerState::Bisecting { current, .. } => current.contains(path),
             PassControllerState::Success { .. } => {
@@ -205,6 +226,7 @@ impl PassController {
         match worklist.pop() {
             Some(next) => {
                 *current = next.into_iter().collect();
+                trace!(?current, "current working set: ");
             }
             None => {
                 self.state = PassControllerState::Success;

src/processor/mod.rs

@@ -187,6 +187,7 @@ impl Minimizer {
         // The logic for bisecting down lives in PassController.
 
         let mut checker = PassController::new(self.options.clone());
+        let mut initial_pass = true;
         loop {
             let mut change = file.try_change(changes)?;
             let (_, krate) = change.before_content();
@@ -213,14 +214,17 @@ impl Minimizer {
                     }
                 }
                 ProcessState::NoChange => {
-                    if self.options.no_color {
+                    if !initial_pass {
-                        info!("{file:?}: After {}: no changes", pass.name());
+                        if self.options.no_color {
-                    } else {
+                            info!("{file:?}: After {}: no changes", pass.name());
-                        info!("{file:?}: After {}: {}", pass.name(), "no changes".yellow());
+                        } else {
+                            info!("{file:?}: After {}: {}", pass.name(), "no changes".yellow());
+                        }
                     }
                     checker.no_change();
                 }
             }
+            initial_pass = false;
 
             if self.cancel.load(Ordering::SeqCst) {
                 info!("Exiting early.");