From 2f885257e62a05d2d60991737576a346e974bdf3 Mon Sep 17 00:00:00 2001 From: moxian Date: Sun, 30 Mar 2025 21:41:37 -0700 Subject: [PATCH] Fix delete-unused-functions panics The pass used to (?) track invalidated files itself, but now that functionality has been moved up one level, but also kinda not really. So here we clarify this by: - making reaper not care about tracking invalidated files anymore - making processor yes care about tracking invalidated files, and ensuring that it does not call process_file again after gettin ProcessState::FileInvalidated, as it advertizes to do. --- src/processor/mod.rs | 10 +++++----- src/processor/reaper.rs | 31 ++++++++----------------------- 2 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/processor/mod.rs b/src/processor/mod.rs index 2843829..3b7dadc 100644 --- a/src/processor/mod.rs +++ b/src/processor/mod.rs @@ -19,7 +19,7 @@ pub(crate) trait Pass { /// 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. + /// before calling this function on the same file again. fn process_file( &mut self, krate: &mut syn::File, @@ -203,14 +203,14 @@ impl Minimizer { if after.reproduces_issue() { change.commit(); checker.reproduces(); + if has_made_change == ProcessState::FileInvalidated { + invalidated_files.insert(file); + break; + } } else { change.rollback()?; checker.does_not_reproduce(); } - - if has_made_change == ProcessState::FileInvalidated { - invalidated_files.insert(file); - } } ProcessState::NoChange => { if self.options.no_color { diff --git a/src/processor/reaper.rs b/src/processor/reaper.rs index 5361d15..e5953c9 100644 --- a/src/processor/reaper.rs +++ b/src/processor/reaper.rs @@ -7,11 +7,7 @@ use anyhow::{Context, Result}; use proc_macro2::Span; use quote::ToTokens; use rustfix::{diagnostics::Diagnostic, Suggestion}; -use std::{ - collections::{HashMap, HashSet}, - ops::Range, - path::{Path, PathBuf}, -}; +use std::{collections::HashMap, ops::Range, path::Path}; use syn::{visit_mut::VisitMut, ImplItem, Item}; fn file_for_suggestion(suggestion: &Suggestion) -> &Path { @@ -103,16 +99,11 @@ impl Minimizer { struct DeleteUnusedFunctions { diags: Vec, build: Build, - invalid: HashSet, } impl DeleteUnusedFunctions { fn new(build: Build, diags: Vec) -> Self { - DeleteUnusedFunctions { - diags, - build, - invalid: HashSet::new(), - } + DeleteUnusedFunctions { diags, build } } } @@ -120,7 +111,6 @@ impl Pass for DeleteUnusedFunctions { fn refresh_state(&mut self) -> Result<()> { let (diags, _) = self.build.get_diags().context("getting diagnostics")?; self.diags = diags; - self.invalid.clear(); Ok(()) } @@ -130,18 +120,9 @@ impl Pass for DeleteUnusedFunctions { file: &SourceFile, checker: &mut super::PassController, ) -> ProcessState { - assert!( - !self.invalid.contains(file.path_no_fs_interact()), - "processing with invalid state" - ); - let mut visitor = FindUnusedFunction::new(file, self.diags.iter(), checker); visitor.visit_file_mut(krate); - if visitor.process_state == ProcessState::FileInvalidated { - self.invalid.insert(file.path_no_fs_interact().to_owned()); - } - visitor.process_state } @@ -235,8 +216,12 @@ impl<'a> FindUnusedFunction<'a> { match span_matches { 0 => true, 1 => { - self.process_state = ProcessState::FileInvalidated; - !self.checker.can_process(&self.current_path) + if self.checker.can_process(&self.current_path) { + self.process_state = ProcessState::FileInvalidated; + !self.checker.can_process(&self.current_path) + } else { + true + } } _ => { panic!("multiple dead_code spans matched identifier: {span_matches}.");