perf(hir): O(1) scope resolution — index local bindings by name (#5267)

[proggeramlug]

Summary

Fixes #5267. Lowering resolved every identifier by a reverse linear scan of the per-scope locals stack, so a scope with N bindings and N references lowered in O(n²). Large minified/bundled JS (tens of thousands of bindings/refs in one module or wrapper-function scope) stalled check-lower for minutes and got killed with no diagnostic.

Change

• New Locals newtype (crates/perry-hir/src/lower/locals.rs) wraps the ordered (name, id, type) stack with a side name → ascending positions index. The innermost binding for a name is the back of its position list, so lookup/lookup_index/lookup_type are O(1). Position-changing ops (push/drain_from/extend/remove) maintain the index; read/in-place ops reach the slice via Deref/DerefMut (&mut [..], not &mut Vec, so the index can't be bypassed by callers).
• Replaced the per-declaration linear scans on the var/fn-hoisting and var-redeclaration paths (block.rs, expr_function.rs, var_decl.rs, stmt.rs) with O(1) lookup_index_in_scope (max position == innermost binding, so "exists idx ≥ mark" iff "max ≥ mark") and an iter_named + type_mut_at patch-in-place (no second re-find by id).

Measured (perry check, single large scope)

N (bindings + refs)	before	after
40 000 (var)	2.23 s	0.12 s
160 000 (var)	killed (extrapolated >600 s)	0.52 s

Scaling is now linear (40k→80k→160k = 0.13→0.25→0.52 s) for both var and let/const.

Tests

• All perry-hir unit tests (200+) and perry CLI integration tests (560+) pass.
• var hoisting, redeclaration, param/block shadowing, closure-captured hoisted var, and nested-var-in-compound-stmt verified byte-identical to node --experimental-strip-types.

Per request, this PR intentionally omits the version bump and CHANGELOG entry.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor
  • Optimized internal compilation performance by replacing linear variable lookup operations with indexed lookups, enabling faster variable name resolution during the compilation process.

[coderabbitai]

📝 Walkthrough

Walkthrough

Introduces a new Locals struct (crates/perry-hir/src/lower/locals.rs) that maintains an authoritative entries Vec alongside a HashMap<String, Vec<usize>> name-to-positions index for O(1) innermost-binding lookups. LoweringContext.locals is retyped from a plain Vec to Locals, and all scope-lookup call sites across the lowering pipeline are migrated to the new indexed API methods.

Changes

O(1) Scope-Local Binding Lookup

Layer / File(s)	Summary
Locals struct and API crates/perry-hir/src/lower/locals.rs, crates/perry-hir/src/lower/mod.rs	New Locals struct with entries: Vec<(String, LocalId, Type)> and HashMap<String, Vec<usize>> index. Provides push, lookup_index, lookup, lookup_type, lookup_type_mut, lookup_index_in_scope, iter_named, type_mut_at, drain_from, extend, remove, reindex, and Deref/DerefMut to [(String, LocalId, Type)]. Module re-exports Locals as pub(crate).
LoweringContext field and method rewiring crates/perry-hir/src/lower/lowering_context.rs, crates/perry-hir/src/lower/context.rs	locals field retyped from Vec<(String, LocalId, Type)> to crate::lower::Locals. Constructor uses Locals::new(). lookup_local, lookup_local_index, and lookup_local_type delegate to self.locals.lookup, lookup_index, lookup_type. Scope unwinding in exit_scope and pop_block_scope switches from drain(mark..) to drain_from(mark).
Call-site migration across hoisting pre-passes and destructuring crates/perry-hir/src/lower/expr_function.rs, crates/perry-hir/src/lower/stmt.rs, crates/perry-hir/src/lower_decl/block.rs, crates/perry-hir/src/destructuring/var_decl.rs	All iter().enumerate().rev().any/find(...) reverse-scan patterns replaced: var/function/let/const hoist pre-passes use lookup_index_in_scope; emit_class_expression_value_binding uses iter_named + type_mut_at; lower_var_decl_with_destructuring uses lookup_type_mut and type_mut_at; pre_register_forward_captured_lets and predefine_var_bindings_in_function_body use lookup_index_in_scope.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• PerryTS/perry#5220: Also modifies lower_var_decl_with_destructuring in destructuring/var_decl.rs, touching the same local-type reuse logic that this PR refactors to use indexed lookups.
• PerryTS/perry#5125: Introduces the pre_register_forward_captured_lets and predefine_var_bindings_in_function_body functions in lower_decl/block.rs whose "already in scope" checks are migrated to lookup_index_in_scope in this PR.

Poem

🐇 In the warren's dusty stacks,
I once searched every den and track,
But now my index lights the way—
O(1) hops instead of n per day!
No more scanning, no more dread,
Just check the HashMap straight ahead. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: optimizing scope resolution from O(n²) to O(1) by indexing local bindings by name.
Description check	✅ Passed	The description comprehensively covers the summary, changes, related issue, test plan with checkboxes, and checklist items per the template.
Linked Issues check	✅ Passed	The PR successfully addresses issue #5267 by implementing O(1) scope resolution with a name-indexed Locals container, replacing linear scans with hash-map lookups, and achieving measured performance improvements from O(n²) to linear scaling.
Out of Scope Changes check	✅ Passed	All changes are directly related to the stated objective of optimizing scope resolution: new Locals container, refactored lookups in var/function hoisting and redeclaration paths, and internal context refactoring to use the new structure.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch worktree-fix-5267-scope-resolution

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-hir/src/lower/locals.rs`:
- Around line 171-174: The DerefMut implementation for Locals currently exposes
mutable access to the entries slice, allowing callers to reorder entries via
swap operations or modify string names directly. This breaks the synchronization
between the entries vector and the index map, causing lookup methods to return
stale or incorrect bindings. Remove the DerefMut implementation for Locals
entirely, or if mutable access is needed, provide explicit controlled methods on
Locals that update both entries and index together, ensuring all mutations that
could affect the index invariant go through Locals methods only.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d7fdcb5d-f2b6-478f-afd5-922ce0a4d0e7

📥 Commits

Reviewing files that changed from the base of the PR and between 392af84 and ddef8e8.

📒 Files selected for processing (8)

• crates/perry-hir/src/destructuring/var_decl.rs
• crates/perry-hir/src/lower/context.rs
• crates/perry-hir/src/lower/expr_function.rs
• crates/perry-hir/src/lower/locals.rs
• crates/perry-hir/src/lower/lowering_context.rs
• crates/perry-hir/src/lower/mod.rs
• crates/perry-hir/src/lower/stmt.rs
• crates/perry-hir/src/lower_decl/block.rs

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Lock down mutable slice access to preserve Locals index invariants.

Line 171 currently exposes &mut [(String, LocalId, Type)]. That still permits reordering (swap/slice ops) and renaming (entry.0), which can desynchronize index from entries and make lookup* return incorrect bindings. Please keep all index-affecting mutations behind Locals methods only.

Proposed direction

-impl DerefMut for Locals {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.entries
-    }
-}
+// Remove DerefMut to prevent index-bypassing mutations.
+// Keep explicit mutation APIs (`push`, `drain_from`, `extend`, `remove`,
+// `lookup_type_mut`, `type_mut_at`) as the only writable surface.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-hir/src/lower/locals.rs` around lines 171 - 174, The DerefMut
implementation for Locals currently exposes mutable access to the entries slice,
allowing callers to reorder entries via swap operations or modify string names
directly. This breaks the synchronization between the entries vector and the
index map, causing lookup methods to return stale or incorrect bindings. Remove
the DerefMut implementation for Locals entirely, or if mutable access is needed,
provide explicit controlled methods on Locals that update both entries and index
together, ensuring all mutations that could affect the index invariant go
through Locals methods only.