Background
An external code audit flagged several HIR/IR-walking helpers that are copy-pasted across files. Each copy must be hand-updated whenever a new Expr/HIR node variant is added — and when one copy is missed, the result is a silent miscompile, not a build error. This is the same bug-class behind several recent fixes (e.g. #5143 FuncId-collision from a transform scan missing class field-init closures).
Verified against main (v0.5.1176).
Confirmed duplication
compute_max_local_id — 5 copies
crates/perry-transform/src/generator/id_scan.rs:6 (canonical, already pub)crates/perry-transform/src/finally_inline.rs:167crates/perry-transform/src/async_to_generator.rs:626crates/perry-transform/src/unroll/mod.rs:671crates/perry-transform/src/state_desugar.rs:175
compute_max_func_id — ~4 copies
crates/perry-transform/src/generator/id_scan.rs:232 (canonical, pub)crates/perry-transform/src/state_desugar.rs:209crates/perry-transform/src/async_to_generator.rs:214 (compute_max_func_id_module)crates/perry-transform/src/unroll/mod.rs:873
The copies carry a comment about "avoiding a pub-visibility bump," but the canonical versions are already pub, so the copies can simply be deleted in favor of importing the canonical ones.
class_computed_member_registration_expr — 4 byte-identical copies
crates/perry-hir/src/lower_decl/class_computed.rs:16 (canonical, pub(crate))crates/perry-hir/src/lower/lower_expr.rs:43crates/perry-hir/src/lower/module_decl.rs:17crates/perry-hir/src/lower/stmt.rs:17
(The latter three are md5-identical; route them all through the canonical class_computed.rs.)
Related exhaustiveness risk (same bug-class)
collect_assigned_locals_expr (crates/perry-hir/src/analysis.rs:330, ~1121 lines) ends in a catch-all _ => {} at analysis.rs:1451. Unlike collect_local_refs_expr / remap_local_ids_in_expr (which delegate child descent to the exhaustive walk_expr_children walker), this one manually enumerates variants — so a newly-added Expr variant containing a nested LocalSet silently won't have its assigned locals collected. Converting it to use the exhaustive walker for child descent (keeping only the assignment-tracking arms custom) would close the gap.
Audit also flagged a 5× module-walking duplication in crates/perry-codegen/src/codegen/mod.rs (walking functions + class methods/ctors/getters/setters/static-methods/computed-members), proposing a for_each_body_in_module(hir, &mut |body| …) helper. Not independently line-verified here, but the same motivation applies.
Proposed fix
- Delete the
compute_max_local_id / compute_max_func_id copies; import the canonical pub versions from generator::id_scan.
- Collapse the 4
class_computed_member_registration_expr copies to the canonical lower_decl::class_computed one.
- Make
collect_assigned_locals_expr delegate child descent to walk_expr_children.
- (Optional) Extract a
for_each_body_in_module iterator for the codegen walks.
Why not bundled into a fix PR
These are pure refactors (no behavior change) and orthogonal to correctness fixes — kept separate to keep diffs reviewable. Surfaced alongside #5292 (which fixed the one reproducible correctness bug from the same audit).
Background
An external code audit flagged several HIR/IR-walking helpers that are copy-pasted across files. Each copy must be hand-updated whenever a new
Expr/HIR node variant is added — and when one copy is missed, the result is a silent miscompile, not a build error. This is the same bug-class behind several recent fixes (e.g. #5143 FuncId-collision from a transform scan missing class field-init closures).Verified against
main(v0.5.1176).Confirmed duplication
compute_max_local_id— 5 copiescrates/perry-transform/src/generator/id_scan.rs:6(canonical, alreadypub)crates/perry-transform/src/finally_inline.rs:167crates/perry-transform/src/async_to_generator.rs:626crates/perry-transform/src/unroll/mod.rs:671crates/perry-transform/src/state_desugar.rs:175compute_max_func_id— ~4 copiescrates/perry-transform/src/generator/id_scan.rs:232(canonical,pub)crates/perry-transform/src/state_desugar.rs:209crates/perry-transform/src/async_to_generator.rs:214(compute_max_func_id_module)crates/perry-transform/src/unroll/mod.rs:873The copies carry a comment about "avoiding a pub-visibility bump," but the canonical versions are already
pub, so the copies can simply be deleted in favor of importing the canonical ones.class_computed_member_registration_expr— 4 byte-identical copiescrates/perry-hir/src/lower_decl/class_computed.rs:16(canonical,pub(crate))crates/perry-hir/src/lower/lower_expr.rs:43crates/perry-hir/src/lower/module_decl.rs:17crates/perry-hir/src/lower/stmt.rs:17(The latter three are md5-identical; route them all through the canonical
class_computed.rs.)Related exhaustiveness risk (same bug-class)
collect_assigned_locals_expr(crates/perry-hir/src/analysis.rs:330, ~1121 lines) ends in a catch-all_ => {}atanalysis.rs:1451. Unlikecollect_local_refs_expr/remap_local_ids_in_expr(which delegate child descent to the exhaustivewalk_expr_childrenwalker), this one manually enumerates variants — so a newly-addedExprvariant containing a nestedLocalSetsilently won't have its assigned locals collected. Converting it to use the exhaustive walker for child descent (keeping only the assignment-tracking arms custom) would close the gap.Audit also flagged a 5× module-walking duplication in
crates/perry-codegen/src/codegen/mod.rs(walking functions + class methods/ctors/getters/setters/static-methods/computed-members), proposing afor_each_body_in_module(hir, &mut |body| …)helper. Not independently line-verified here, but the same motivation applies.Proposed fix
compute_max_local_id/compute_max_func_idcopies; import the canonicalpubversions fromgenerator::id_scan.class_computed_member_registration_exprcopies to the canonicallower_decl::class_computedone.collect_assigned_locals_exprdelegate child descent towalk_expr_children.for_each_body_in_moduleiterator for the codegen walks.Why not bundled into a fix PR
These are pure refactors (no behavior change) and orthogonal to correctness fixes — kept separate to keep diffs reviewable. Surfaced alongside #5292 (which fixed the one reproducible correctness bug from the same audit).