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.
This commit is contained in:
moxian 2025-03-30 21:41:37 -07:00 committed by nora
parent b44cd4e6eb
commit 2f885257e6
2 changed files with 13 additions and 28 deletions

View file

@ -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 /// 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` /// `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( fn process_file(
&mut self, &mut self,
krate: &mut syn::File, krate: &mut syn::File,
@ -203,14 +203,14 @@ impl Minimizer {
if after.reproduces_issue() { if after.reproduces_issue() {
change.commit(); change.commit();
checker.reproduces(); checker.reproduces();
if has_made_change == ProcessState::FileInvalidated {
invalidated_files.insert(file);
break;
}
} else { } else {
change.rollback()?; change.rollback()?;
checker.does_not_reproduce(); checker.does_not_reproduce();
} }
if has_made_change == ProcessState::FileInvalidated {
invalidated_files.insert(file);
}
} }
ProcessState::NoChange => { ProcessState::NoChange => {
if self.options.no_color { if self.options.no_color {

View file

@ -7,11 +7,7 @@ use anyhow::{Context, Result};
use proc_macro2::Span; use proc_macro2::Span;
use quote::ToTokens; use quote::ToTokens;
use rustfix::{diagnostics::Diagnostic, Suggestion}; use rustfix::{diagnostics::Diagnostic, Suggestion};
use std::{ use std::{collections::HashMap, ops::Range, path::Path};
collections::{HashMap, HashSet},
ops::Range,
path::{Path, PathBuf},
};
use syn::{visit_mut::VisitMut, ImplItem, Item}; use syn::{visit_mut::VisitMut, ImplItem, Item};
fn file_for_suggestion(suggestion: &Suggestion) -> &Path { fn file_for_suggestion(suggestion: &Suggestion) -> &Path {
@ -103,16 +99,11 @@ impl Minimizer {
struct DeleteUnusedFunctions { struct DeleteUnusedFunctions {
diags: Vec<Diagnostic>, diags: Vec<Diagnostic>,
build: Build, build: Build,
invalid: HashSet<PathBuf>,
} }
impl DeleteUnusedFunctions { impl DeleteUnusedFunctions {
fn new(build: Build, diags: Vec<Diagnostic>) -> Self { fn new(build: Build, diags: Vec<Diagnostic>) -> Self {
DeleteUnusedFunctions { DeleteUnusedFunctions { diags, build }
diags,
build,
invalid: HashSet::new(),
}
} }
} }
@ -120,7 +111,6 @@ impl Pass for DeleteUnusedFunctions {
fn refresh_state(&mut self) -> Result<()> { fn refresh_state(&mut self) -> Result<()> {
let (diags, _) = self.build.get_diags().context("getting diagnostics")?; let (diags, _) = self.build.get_diags().context("getting diagnostics")?;
self.diags = diags; self.diags = diags;
self.invalid.clear();
Ok(()) Ok(())
} }
@ -130,18 +120,9 @@ impl Pass for DeleteUnusedFunctions {
file: &SourceFile, file: &SourceFile,
checker: &mut super::PassController, checker: &mut super::PassController,
) -> ProcessState { ) -> ProcessState {
assert!(
!self.invalid.contains(file.path_no_fs_interact()),
"processing with invalid state"
);
let mut visitor = FindUnusedFunction::new(file, self.diags.iter(), checker); let mut visitor = FindUnusedFunction::new(file, self.diags.iter(), checker);
visitor.visit_file_mut(krate); visitor.visit_file_mut(krate);
if visitor.process_state == ProcessState::FileInvalidated {
self.invalid.insert(file.path_no_fs_interact().to_owned());
}
visitor.process_state visitor.process_state
} }
@ -235,8 +216,12 @@ impl<'a> FindUnusedFunction<'a> {
match span_matches { match span_matches {
0 => true, 0 => true,
1 => { 1 => {
self.process_state = ProcessState::FileInvalidated; if self.checker.can_process(&self.current_path) {
!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}."); panic!("multiple dead_code spans matched identifier: {span_matches}.");