From d7737574abf6adbabca3ff45c22d77b9d0bbc5f5 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 8 Jan 2025 19:04:32 +0100 Subject: [PATCH] Remove the newly-added clauses if not used --- charon/src/ast/krate.rs | 33 ++++++ charon/src/ast/visitor.rs | 4 +- charon/src/ids/vector.rs | 10 +- charon/src/transform/mod.rs | 4 + .../transform/remove_unused_self_clause.rs | 100 ++++++++++++++++++ charon/tests/ui/assoc-const-with-generics.out | 9 +- charon/tests/ui/rename_attribute.out | 4 +- charon/tests/ui/traits.out | 13 +-- 8 files changed, 154 insertions(+), 23 deletions(-) create mode 100644 charon/src/transform/remove_unused_self_clause.rs diff --git a/charon/src/ast/krate.rs b/charon/src/ast/krate.rs index 5f985792..c7960f89 100644 --- a/charon/src/ast/krate.rs +++ b/charon/src/ast/krate.rs @@ -218,6 +218,17 @@ impl<'ctx> AnyTransItem<'ctx> { } } + /// Get information about the parent of this item, if any. + pub fn parent_info(&self) -> &'ctx ItemKind { + match self { + AnyTransItem::Fun(d) => &d.kind, + AnyTransItem::Global(d) => &d.kind, + AnyTransItem::Type(_) | AnyTransItem::TraitDecl(_) | AnyTransItem::TraitImpl(_) => { + &ItemKind::Regular + } + } + } + /// See [`GenericParams::identity_args`]. pub fn identity_args(&self) -> GenericArgs { self.generic_params() @@ -235,6 +246,17 @@ impl<'ctx> AnyTransItem<'ctx> { AnyTransItem::TraitImpl(d) => visitor.visit(d), } } + + /// Visit all occurrences of that type inside `self`, in pre-order traversal. + pub fn dyn_visit(&self, f: impl FnMut(&T)) { + match *self { + AnyTransItem::Type(d) => d.dyn_visit(f), + AnyTransItem::Fun(d) => d.dyn_visit(f), + AnyTransItem::Global(d) => d.dyn_visit(f), + AnyTransItem::TraitDecl(d) => d.dyn_visit(f), + AnyTransItem::TraitImpl(d) => d.dyn_visit(f), + } + } } impl<'ctx> AnyTransItemMut<'ctx> { @@ -270,6 +292,17 @@ impl<'ctx> AnyTransItemMut<'ctx> { AnyTransItemMut::TraitImpl(d) => visitor.visit(*d), } } + + /// Visit all occurrences of that type inside `self`, in pre-order traversal. + pub fn dyn_visit_mut(&mut self, f: impl FnMut(&mut T)) { + match self { + AnyTransItemMut::Type(d) => d.dyn_visit_mut(f), + AnyTransItemMut::Fun(d) => d.dyn_visit_mut(f), + AnyTransItemMut::Global(d) => d.dyn_visit_mut(f), + AnyTransItemMut::TraitDecl(d) => d.dyn_visit_mut(f), + AnyTransItemMut::TraitImpl(d) => d.dyn_visit_mut(f), + } + } } impl fmt::Display for TranslatedCrate { diff --git a/charon/src/ast/visitor.rs b/charon/src/ast/visitor.rs index 7930530e..33b4e642 100644 --- a/charon/src/ast/visitor.rs +++ b/charon/src/ast/visitor.rs @@ -46,7 +46,7 @@ use index_vec::Idx; FnOperand, FunId, FunIdOrTraitMethodRef, FunSig, ImplElem, IntegerTy, Literal, LiteralTy, llbc_ast::Block, llbc_ast::ExprBody, llbc_ast::RawStatement, llbc_ast::Switch, Locals, Name, NullOp, Opaque, Operand, PathElem, Place, PlaceKind, ProjectionElem, RawConstantExpr, - RefKind, RegionId, RegionVar, Rvalue, ScalarValue, TraitClause, TraitClauseId, TraitItemName, + RefKind, RegionId, RegionVar, Rvalue, ScalarValue, TraitItemName, TraitRefKind, TraitTypeConstraint, TranslatedCrate, TyKind, TypeDeclKind, TypeId, TypeVar, TypeVarId, ullbc_ast::BlockData, ullbc_ast::BlockId, ullbc_ast::ExprBody, ullbc_ast::RawStatement, ullbc_ast::RawTerminator, ullbc_ast::SwitchTargets, ullbc_ast::Terminator, @@ -64,7 +64,7 @@ use index_vec::Idx; override( DeBruijnId, Ty, Region, ConstGeneric, TraitRef, FunDeclRef, GlobalDeclRef, TraitDeclRef, TraitImplRef, - GenericArgs, GenericParams, + GenericArgs, GenericParams, TraitClause, TraitClauseId, for DeBruijnVar, for RegionBinder, for Binder, diff --git a/charon/src/ids/vector.rs b/charon/src/ids/vector.rs index 24ec8a05..154b2065 100644 --- a/charon/src/ids/vector.rs +++ b/charon/src/ids/vector.rs @@ -84,7 +84,7 @@ where self.real_len += 1; } - /// Remove the value from this slot. + /// Remove the value from this slot, leaving other ids unchanged. pub fn remove(&mut self, id: I) -> Option { if self.vector[id].is_some() { self.real_len -= 1; @@ -92,6 +92,14 @@ where self.vector[id].take() } + /// Remove the value from this slot, shifting other ids as needed. + pub fn remove_and_shift_ids(&mut self, id: I) -> Option { + if self.vector[id].is_some() { + self.real_len -= 1; + } + self.vector.remove(id) + } + pub fn push(&mut self, x: T) -> I { self.real_len += 1; self.vector.push(Some(x)) diff --git a/charon/src/transform/mod.rs b/charon/src/transform/mod.rs index f4fa13ce..8059d572 100644 --- a/charon/src/transform/mod.rs +++ b/charon/src/transform/mod.rs @@ -22,6 +22,7 @@ pub mod remove_dynamic_checks; pub mod remove_nops; pub mod remove_read_discriminant; pub mod remove_unused_locals; +pub mod remove_unused_self_clause; pub mod reorder_decls; pub mod simplify_constants; pub mod skip_trait_refs_when_known; @@ -45,6 +46,9 @@ pub static INITIAL_CLEANUP_PASSES: &[Pass] = &[ // # Micro-pass: filter the trait impls that were marked invisible since we couldn't filter // them out earlier. NonBody(&filter_invisible_trait_impls::Transform), + // # Micro-pass: remove the explicit `Self: Trait` clause of methods/assco const declaration + // items if they're not used. This simplifies the graph of dependencies between definitions. + NonBody(&remove_unused_self_clause::Transform), // # Micro-pass: whenever we call a trait method on a known type, refer to the method `FunDecl` // directly instead of going via a `TraitRef`. This is done before `reorder_decls` to remove // some sources of mutual recursion. diff --git a/charon/src/transform/remove_unused_self_clause.rs b/charon/src/transform/remove_unused_self_clause.rs new file mode 100644 index 00000000..6d481bc1 --- /dev/null +++ b/charon/src/transform/remove_unused_self_clause.rs @@ -0,0 +1,100 @@ +//! We have added an explicit `Self: Trait` clause to the function and global items that correspond +//! to a trait method/associated const declaration. This pass removes the clause in question if it +//! is not used by the item. +use derive_generic_visitor::*; +use std::collections::HashSet; + +use crate::ast::*; + +use super::{ctx::TransformPass, TransformCtx}; + +struct FoundClause; + +struct UsesClauseVisitor(TraitClauseId); +impl Visitor for UsesClauseVisitor { + type Break = FoundClause; +} + +/// Visit an item looking for uses of the given clause. +impl VisitAst for UsesClauseVisitor { + fn visit_trait_clause_id(&mut self, x: &TraitClauseId) -> ControlFlow { + if *x == self.0 { + Break(FoundClause) + } else { + Continue(()) + } + } + fn visit_trait_clause(&mut self, _: &TraitClause) -> ControlFlow { + // Don't look inside the clause declaration as this will always contain the + // `TraitClauseId`. + Continue(()) + } + fn visit_fun_decl(&mut self, x: &FunDecl) -> ControlFlow { + if let Err(Opaque) = x.body { + // For function without bodies we can't know whether the clause is used so we err on + // the side of it being used. + return Break(FoundClause); + } + self.visit_inner(x) + } +} + +pub struct Transform; +impl TransformPass for Transform { + fn transform_ctx(&self, ctx: &mut TransformCtx) { + let self_clause_id = TraitClauseId::from_raw(0); + let mut doesnt_use_self: HashSet = Default::default(); + + // We explore only items with an explicit `Self` clause, namely method and associated const + // declarations. + for tdecl in &ctx.translated.trait_decls { + let methods = tdecl + .required_methods + .iter() + .chain(tdecl.provided_methods.iter()) + .map(|(_, bound_fn)| bound_fn.skip_binder.id); + // For consts, we need to explore the corresponding initializer body. + let consts = tdecl + .const_defaults + .iter() + .filter_map(|(_, x)| ctx.translated.global_decls.get(x.id)) + .map(|gdecl| gdecl.init); + let funs = methods + .chain(consts) + .filter_map(|id: FunDeclId| ctx.translated.fun_decls.get(id)); + for fun in funs { + match fun.drive(&mut UsesClauseVisitor(self_clause_id)) { + Continue(()) => { + doesnt_use_self.insert(fun.def_id.into()); + if let Some(gid) = fun.is_global_initializer { + doesnt_use_self.insert(gid.into()); + } + } + Break(FoundClause) => {} + } + } + } + + // In each item, remove the first clause and renumber the others. + for &id in &doesnt_use_self { + let Some(mut item) = ctx.translated.get_item_mut(id) else { + continue; + }; + item.generic_params() + .trait_clauses + .remove_and_shift_ids(self_clause_id); + item.dyn_visit_mut(|clause_id: &mut TraitClauseId| { + *clause_id = TraitClauseId::from_usize(clause_id.index() - 1); + }); + } + + // Update any `GenericArgs` destined for the items we just changed. + ctx.translated.dyn_visit_mut(|args: &mut GenericArgs| { + if let GenericsSource::Item(target_id) = args.target + && doesnt_use_self.contains(&target_id) + { + args.trait_refs.remove_and_shift_ids(self_clause_id); + } + }); + } +} diff --git a/charon/tests/ui/assoc-const-with-generics.out b/charon/tests/ui/assoc-const-with-generics.out index 0740684d..3b34ff3b 100644 --- a/charon/tests/ui/assoc-const-with-generics.out +++ b/charon/tests/ui/assoc-const-with-generics.out @@ -94,8 +94,6 @@ where } fn test_crate::HasDefaultLen::LEN() -> usize -where - [@TraitClause0]: test_crate::HasDefaultLen, { let @0: usize; // return @@ -103,10 +101,7 @@ where return } -global test_crate::HasDefaultLen::LEN: usize - where - [@TraitClause0]: test_crate::HasDefaultLen, - = test_crate::HasDefaultLen::LEN() +global test_crate::HasDefaultLen::LEN: usize = test_crate::HasDefaultLen::LEN() trait test_crate::HasDefaultLen { @@ -115,7 +110,7 @@ trait test_crate::HasDefaultLen impl test_crate::{impl test_crate::HasDefaultLen for Array<(), const N : usize>}#4 : test_crate::HasDefaultLen, const N : usize> { - const LEN = test_crate::HasDefaultLen::LEN, const N : usize>[test_crate::{impl test_crate::HasDefaultLen for Array<(), const N : usize>}#4] + const LEN = test_crate::HasDefaultLen::LEN, const N : usize> } fn test_crate::{impl test_crate::HasDefaultLen for Array}#5::LEN() -> usize diff --git a/charon/tests/ui/rename_attribute.out b/charon/tests/ui/rename_attribute.out index 789abe0b..bd361b91 100644 --- a/charon/tests/ui/rename_attribute.out +++ b/charon/tests/ui/rename_attribute.out @@ -3,7 +3,7 @@ trait test_crate::BoolTrait { fn get_bool<'_0> = test_crate::BoolTrait::get_bool<'_0_0, Self>[Self] - fn ret_true<'_0> = test_crate::BoolTrait::ret_true<'_0_0, Self>[Self] + fn ret_true<'_0> = test_crate::BoolTrait::ret_true<'_0_0, Self> } fn test_crate::{impl test_crate::BoolTrait for bool}::get_bool<'_0>(@1: &'_0 (bool)) -> bool @@ -23,8 +23,6 @@ impl test_crate::{impl test_crate::BoolTrait for bool} : test_crate::BoolTrait fn test_crate::BoolTrait::ret_true<'_0, Self>(@1: &'_0 (Self)) -> bool -where - [@TraitClause0]: test_crate::BoolTrait, { let @0: bool; // return let self@1: &'_ (Self); // arg #1 diff --git a/charon/tests/ui/traits.out b/charon/tests/ui/traits.out index 06a1fdad..b4f49491 100644 --- a/charon/tests/ui/traits.out +++ b/charon/tests/ui/traits.out @@ -3,7 +3,7 @@ trait test_crate::BoolTrait { fn get_bool<'_0> = test_crate::BoolTrait::get_bool<'_0_0, Self>[Self] - fn ret_true<'_0> = test_crate::BoolTrait::ret_true<'_0_0, Self>[Self] + fn ret_true<'_0> = test_crate::BoolTrait::ret_true<'_0_0, Self> } fn test_crate::{impl test_crate::BoolTrait for bool}::get_bool<'_0>(@1: &'_0 (bool)) -> bool @@ -21,8 +21,6 @@ impl test_crate::{impl test_crate::BoolTrait for bool} : test_crate::BoolTrait(@1: &'_0 (Self)) -> bool -where - [@TraitClause0]: test_crate::BoolTrait, { let @0: bool; // return let self@1: &'_ (Self); // arg #1 @@ -488,8 +486,6 @@ where } fn test_crate::WithConstTy::LEN2() -> usize -where - [@TraitClause0]: test_crate::WithConstTy, { let @0: usize; // return @@ -497,10 +493,7 @@ where return } -global test_crate::WithConstTy::LEN2: usize - where - [@TraitClause0]: test_crate::WithConstTy, - = test_crate::WithConstTy::LEN2() +global test_crate::WithConstTy::LEN2: usize = test_crate::WithConstTy::LEN2() trait test_crate::WithConstTy { @@ -543,7 +536,7 @@ impl test_crate::{impl test_crate::WithConstTy<32 : usize> for bool}#8 : test_cr parent_clause1 = test_crate::{impl test_crate::ToU64 for u64}#2 parent_clause2 = core::marker::Sized const LEN1 = test_crate::{impl test_crate::WithConstTy<32 : usize> for bool}#8::LEN1 - const LEN2 = test_crate::WithConstTy::LEN2[test_crate::{impl test_crate::WithConstTy<32 : usize> for bool}#8] + const LEN2 = test_crate::WithConstTy::LEN2 type V = u8 type W = u64 fn f<'_0, '_1> = test_crate::{impl test_crate::WithConstTy<32 : usize> for bool}#8::f<'_0_0, '_0_1>