From fdfde615f6defca911e89e0b41c383bcb2dfd234 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sun, 22 Jan 2023 13:19:18 +0100 Subject: [PATCH] item deleter pass --- src/build.rs | 1 + src/lib.rs | 6 ++ src/passes/everybody_loops.rs | 2 + src/passes/item_deleter.rs | 111 ++++++++++++++++++++++++++++++++++ src/passes/mod.rs | 3 +- src/processor/mod.rs | 12 ++++ src/processor/reaper.rs | 8 ++- tests/minimize.rs | 24 +++++++- 8 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 src/passes/item_deleter.rs diff --git a/src/build.rs b/src/build.rs index 6b005ed..f57a603 100644 --- a/src/build.rs +++ b/src/build.rs @@ -315,6 +315,7 @@ impl Build { } } +#[derive(Debug)] pub struct BuildResult { reproduces_issue: bool, no_verify: bool, diff --git a/src/lib.rs b/src/lib.rs index 0070c2d..713a487 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -74,6 +74,10 @@ pub struct Options { #[arg(default_value = "src")] pub path: PathBuf, + /// A comma-seperated list of passes that should be enabled. By default, all passes are enabled. + #[arg(long)] + pub passes: Option, + /// A path to a script that is run to check whether code reproduces. When it exits with code 0, the /// problem reproduces. If `--script-path-lints` isn't set, this script is also run to get lints. /// For lints, the `MINIMIZE_LINTS` environment variable will be set to `1`. @@ -119,6 +123,7 @@ pub fn minimize(options: Options) -> Result<()> { minimizer.run_passes([ passes::Privatize::default().boxed(), passes::EverybodyLoops::default().boxed(), + passes::ItemDeleter::default().boxed(), ])?; minimizer.delete_dead_code().context("deleting dead code")?; @@ -154,6 +159,7 @@ impl Default for Options { env: Vec::new(), project_dir: None, path: PathBuf::from("/the/wrong/path/you/need/to/change/it"), + passes: None, script_path: None, script_path_lints: None, } diff --git a/src/passes/everybody_loops.rs b/src/passes/everybody_loops.rs index 19b7c50..897175f 100644 --- a/src/passes/everybody_loops.rs +++ b/src/passes/everybody_loops.rs @@ -28,6 +28,8 @@ impl VisitMut for Visitor<'_> { [syn::Stmt::Expr(syn::Expr::Loop(syn::ExprLoop { body: loop_body, .. }))] if loop_body.stmts.is_empty() => {} + // Empty bodies are empty already, no need to loopify them. + [] => {} _ if self.checker.can_process(&self.current_path) => { *block = self.loop_expr.clone(); self.process_state = ProcessState::Changed; diff --git a/src/passes/item_deleter.rs b/src/passes/item_deleter.rs new file mode 100644 index 0000000..cb736e7 --- /dev/null +++ b/src/passes/item_deleter.rs @@ -0,0 +1,111 @@ +use quote::ToTokens; +use syn::{ + visit_mut::VisitMut, Item, ItemConst, ItemEnum, ItemMacro, ItemMacro2, ItemMod, ItemStatic, + ItemStruct, ItemTrait, ItemType, ItemUnion, +}; + +use crate::processor::{tracking, Pass, PassController, ProcessState, SourceFile}; + +struct Visitor<'a> { + current_path: Vec, + checker: &'a mut PassController, + process_state: ProcessState, +} + +impl<'a> Visitor<'a> { + fn new(checker: &'a mut PassController) -> Self { + Self { + current_path: Vec::new(), + checker, + process_state: ProcessState::NoChange, + } + } + + fn should_retain_item(&mut self) -> bool { + let can_process = self.checker.can_process(&self.current_path); + if can_process { + self.process_state = ProcessState::Changed; + } + !can_process + } + + fn consider_deleting_item(&mut self, item: &Item) -> bool { + match item { + // N.B. Do not delete ItemFn because that makes testing way harder + // and also the dead_lint should cover it all. + Item::Impl(impl_) => { + self.current_path + .push(impl_.self_ty.clone().into_token_stream().to_string()); + + let should_retain = self.should_retain_item(); + + self.current_path.pop(); + should_retain + } + Item::Struct(ItemStruct { ident, .. }) + | Item::Enum(ItemEnum { ident, .. }) + | Item::Union(ItemUnion { ident, .. }) + | Item::Const(ItemConst { ident, .. }) + | Item::Type(ItemType { ident, .. }) + | Item::Trait(ItemTrait { ident, .. }) + | Item::Macro(ItemMacro { + ident: Some(ident), .. + }) + | Item::Macro2(ItemMacro2 { ident, .. }) + | Item::Static(ItemStatic { ident, .. }) + | Item::Mod(ItemMod { ident, .. }) => { + self.current_path.push(ident.to_string()); + + let should_retain = self.should_retain_item(); + + self.current_path.pop(); + should_retain + } + _ => true, + } + } +} + +impl VisitMut for Visitor<'_> { + fn visit_file_mut(&mut self, file: &mut syn::File) { + file.items + .retain_mut(|item| self.consider_deleting_item(item)); + + syn::visit_mut::visit_file_mut(self, file); + } + + fn visit_item_mod_mut(&mut self, module: &mut syn::ItemMod) { + self.current_path.push(module.ident.to_string()); + + if let Some((_, items)) = &mut module.content { + items.retain(|item| self.consider_deleting_item(item)); + } + + syn::visit_mut::visit_item_mod_mut(self, module); + self.current_path.pop(); + } + + tracking!(visit_item_fn_mut); + tracking!(visit_impl_item_method_mut); + tracking!(visit_item_impl_mut); +} + +#[derive(Default)] +pub struct ItemDeleter; + +impl Pass for ItemDeleter { + fn process_file( + &mut self, + krate: &mut syn::File, + _: &SourceFile, + checker: &mut PassController, + ) -> ProcessState { + let mut visitor = Visitor::new(checker); + visitor.visit_file_mut(krate); + visitor.process_state + } + + fn name(&self) -> &'static str { + "item-deleter" + } +} diff --git a/src/passes/mod.rs b/src/passes/mod.rs index 74973ef..abe6797 100644 --- a/src/passes/mod.rs +++ b/src/passes/mod.rs @@ -1,4 +1,5 @@ mod everybody_loops; +mod item_deleter; mod privatize; -pub use self::{everybody_loops::EverybodyLoops, privatize::Privatize}; +pub use self::{everybody_loops::EverybodyLoops, item_deleter::ItemDeleter, privatize::Privatize}; diff --git a/src/processor/mod.rs b/src/processor/mod.rs index 8c4d07b..c4bf1ea 100644 --- a/src/processor/mod.rs +++ b/src/processor/mod.rs @@ -56,6 +56,15 @@ pub(crate) struct Minimizer { } impl Minimizer { + fn pass_disabled(&self, name: &str) -> bool { + if let Some(passes) = &self.options.passes { + if !passes.split(",").any(|allowed| name == allowed) { + return true; + } + } + false + } + pub(crate) fn new_glob_dir(options: Options, build: Build) -> Result { let path = &options.path; let walk = walkdir::WalkDir::new(path); @@ -102,6 +111,9 @@ impl Minimizer { inital_build.require_reproduction("Initial")?; for mut pass in passes { + if self.pass_disabled(pass.name()) { + continue; + } self.run_pass(&mut *pass)?; } diff --git a/src/processor/reaper.rs b/src/processor/reaper.rs index 7b410da..65e70b4 100644 --- a/src/processor/reaper.rs +++ b/src/processor/reaper.rs @@ -18,8 +18,14 @@ fn file_for_suggestion(suggestion: &Suggestion) -> &str { &suggestion.solutions[0].replacements[0].snippet.file_name } +const PASS_NAME: &str = "delete-unused-functions"; + impl Minimizer { pub fn delete_dead_code(&mut self) -> Result<()> { + if self.pass_disabled(PASS_NAME) { + return Ok(()); + } + let inital_build = self.build.build()?; info!("Before reaper: {inital_build}"); @@ -139,7 +145,7 @@ impl Pass for DeleteUnusedFunctions { } fn name(&self) -> &'static str { - "delete-unused-functions" + PASS_NAME } } diff --git a/tests/minimize.rs b/tests/minimize.rs index c7f3774..cff645c 100644 --- a/tests/minimize.rs +++ b/tests/minimize.rs @@ -47,6 +47,26 @@ fn unused() -> Result<()> { ) } +#[test] +fn impls() -> Result<()> { + // Delete unused impls + run_test( + r##" + pub trait Uwu {} + impl Uwu for () {} + impl Uwu for u8 {} + + fn main() {} + "##, + r##" + fn main() {} + "##, + |opts| { + opts.no_verify = true; + }, + ) +} + #[test] #[cfg_attr(windows, ignore)] fn custom_script_success() -> Result<()> { @@ -61,9 +81,7 @@ fn custom_script_success() -> Result<()> { fn main() {} "##, r##" - fn main() { - loop {} - } + fn main() {} "##, |opts| { opts.script_path = Some(script_path);