Skip to content

fix(hir,runtime): Annex B B.3.3 hoisting precedence + legacy method aliases (#5346)#5394

Merged
proggeramlug merged 1 commit into
mainfrom
fix/5346-b33-hoist-precedence
Jun 18, 2026
Merged

fix(hir,runtime): Annex B B.3.3 hoisting precedence + legacy method aliases (#5346)#5394
proggeramlug merged 1 commit into
mainfrom
fix/5346-b33-hoist-precedence

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Three self-contained, non-eval clusters from the test262 annexB tail (#5346). Builds on the already-merged #5356 (B.3.3 skip cases). All changes fully flip tests green with zero regressions.

annexB parity: 776/1026 (75.6%) → 793/1026 (77.3%), +17 pass.

1. B.3.3 hoisting precedence — global-code/*existing-fn-no-init (8)

When a sloppy block-nested function f(){} and a same-named top-level function f(){} coexist at program scope, F is already in declaredFunctionNames, so B.3.3 must not create a fresh undefined legacy var — the function declaration owns the entry binding.

A non-reassigned top-level function f is otherwise called straight through lookup_func and never bound to the var slot, so the legacy var shadowed it as undefined and f() threw TypeError: value is not a function at entry:

assert.sameValue(f(), 'outer declaration'); // Perry threw here
if (true) function f() { return 'inner declaration'; }
function f() { return 'outer declaration'; }

Fix: seed the legacy-var slot with FuncRef(func_id) (and mark it function-valued) when a same-named top-level function exists. The block-level declaration still overwrites it, so existing-fn-update stays green. lower/lower_module_fn.rs.

2. String.prototype.trimLeft / trimRight (8)

These Annex B legacy names must be the same function objects as trimStart/trimEndtrimLeft === trimStart, .name === "trimStart", real own data properties — not independent thunks (they already worked as method calls, but String.prototype.trimLeft was undefined). Alias the installed property value.

3. Date.prototype.toGMTString (1)

Same legacy-alias requirement: toGMTString === toUTCString. It was installed as a second independent thunk, failing the reference-equality check.

A new install_proto_method_alias helper installs an alias as the same function object with the standard { writable: true, enumerable: false, configurable: true } method descriptor.

Validation

  • Each fix matches node v26 exactly (trim aliases + .name, toGMTString === toUTCString, f() = outer at entry / inner after the block).
  • Full annexB differential: +17 pass, 0 regressions (diffed failure sets before/after).
  • Smoke-tested ordinary function hoisting, reassignment + block-shadow, and the B.3.3-positive (block-only) case — all match Node.

Remaining annexB tail (separate follow-ups, deliberately out of scope)

  • regex (~25): legacy/lookbehind/control escapes — the categorical Rust regex crate gap noted in CLAUDE.md.
  • String.prototype.substr edge cases (~6): length/start coercion + error ordering.
  • escape/unescape not-a-constructor (2): the per-closure non-constructable flag already makes isConstructor correct via the dynamic path, but the literal new escape() codegen special-case bypasses js_new_function_construct — needs a codegen change.
  • B.3.4 var-in-catch (catch-redeclared-*, 4): catch-param vs hoisted-var assignment resolution.
  • dynamic eval (~184): AOT limitation, per the issue.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved Annex B compatibility for legacy JavaScript method aliases by preserving function identity for prototype methods (e.g., Date.prototype.toGMTString now aliases toUTCString).
    • Ensured String.prototype.trimLeft and String.prototype.trimRight reference the same functions as trimStart and trimEnd, respectively.
    • Fixed sloppy-mode hoisting behavior for block-nested function declarations to maintain correct entry-time semantics.

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 36f144e4-a0ca-4aa2-8de0-5db2f2817f13

📥 Commits

Reviewing files that changed from the base of the PR and between 2354f8e and cdca560.

📒 Files selected for processing (5)
  • clusterAg
  • crates/perry-hir/src/lower/lower_module_fn.rs
  • crates/perry-runtime/src/object/date_proto_thunks.rs
  • crates/perry-runtime/src/object/global_this.rs
  • crates/perry-runtime/src/object/string_proto_thunks.rs
🚧 Files skipped from review as they are similar to previous changes (4)
  • crates/perry-runtime/src/object/string_proto_thunks.rs
  • crates/perry-runtime/src/object/date_proto_thunks.rs
  • crates/perry-hir/src/lower/lower_module_fn.rs
  • crates/perry-runtime/src/object/global_this.rs

📝 Walkthrough

Walkthrough

The PR implements two Annex B compliance fixes: in the HIR lowerer, hoisted var slots for block-nested sloppy-mode function declarations are now conditionally seeded with a FuncRef (instead of always Undefined) when a same-named implemented top-level function exists. In the runtime, a new install_proto_method_alias helper ensures legacy aliases (trimLeft, trimRight, toGMTString) are reference-identical to their canonical counterparts.

Changes

Annex B compliance: hoisted var seeding and prototype method aliasing

Layer / File(s) Summary
Annex B.3.3 hoisted var slot initialization
crates/perry-hir/src/lower/lower_module_fn.rs
The prepass in lower_module_full now seeds the hoisted var slot with Expr::FuncRef(func_id) and marks it function-valued when a same-named implemented top-level function is found in functions_with_bodies; otherwise it retains Expr::Undefined.
install_proto_method_alias helper + String/Date Annex B aliases
crates/perry-runtime/src/object/global_this.rs, crates/perry-runtime/src/object/string_proto_thunks.rs, crates/perry-runtime/src/object/date_proto_thunks.rs
Adds install_proto_method_alias that assigns an existing function value to an alias property name with standard built-in descriptor attributes. Wires trimLeft→trimStart, trimRight→trimEnd, and toGMTString→toUTCString through this helper instead of installing independent thunks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

  • PerryTS/perry#5319: Introduces the Annex B.3.3 sloppy-mode block-function hoisting prepass in lower_module_full that this PR builds upon, using that prepass logic to seed the hoisted var slots conditionally.
  • PerryTS/perry#5356: Also modifies Annex B.3.3 handling of the legacy hoisted var for block-nested sloppy functions, specifically adjusting when the var is skipped, which is the complementary decision point to this PR's slot-initialization change.

Poem

🐇 Hop, hop! The slots are seeded right,
No more undefined in the night.
FuncRef blooms where functions roam,
And aliases share a single home—
trimLeft, toGMTString, side by side,
With canonical kin they proudly hide. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: fixing Annex B B.3.3 hoisting precedence and implementing legacy method aliases, with a specific issue reference.
Description check ✅ Passed The description comprehensively covers all three fixes with clear explanations, validation details, code examples, and explicitly defers remaining work, despite missing some optional template sections.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 fix/5346-b33-hoist-precedence

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

…liases (#5346)

Three self-contained, non-eval clusters from the test262 `annexB` tail:

1. B.3.3 hoisting precedence (`global-code/*existing-fn-no-init`, 8 cases).
   When a sloppy block-nested `function f(){}` and a same-named *top-level*
   `function f(){}` coexist at program scope, F is already in
   declaredFunctionNames, so B.3.3 must NOT create a fresh `undefined` legacy
   var — the function declaration owns the entry binding. A non-reassigned
   top-level `function f` is otherwise called straight through `lookup_func`
   and never bound to the var slot, so the legacy var shadowed it as
   `undefined` and `f()` threw `TypeError: value is not a function` at entry.
   Seed the legacy-var slot with `FuncRef(func_id)` (and mark it
   function-valued) when a top-level function of that name exists; the
   block-level declaration still overwrites it (`existing-fn-update` stays
   green). `lower/lower_module_fn.rs`.

2. `String.prototype.trimLeft`/`trimRight` (8 cases). These Annex B legacy
   names must be the SAME function objects as `trimStart`/`trimEnd`
   (`trimLeft === trimStart`, `.name === "trimStart"`, real own data
   properties), not independent thunks. Alias the installed property value.

3. `Date.prototype.toGMTString` (1 case). Same legacy-alias requirement:
   `toGMTString === toUTCString`. Was installed as a second independent thunk.

New `install_proto_method_alias` helper installs an alias as the same function
object with the standard `{ writable, !enumerable, configurable }` descriptor.

test262 `annexB` (node v26 differential, `--all-features`): 776/1026 (75.6%) ->
793/1026 (77.3%); +17 pass, zero regressions. Remaining non-eval tail is the
regex categorical gap (Rust `regex` crate), `String.prototype.substr` edge
cases, `escape`/`unescape` not-a-constructor (needs the literal-`new` codegen
path to honor [[Construct]]), and B.3.4 var-in-catch — separate follow-ups.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@proggeramlug proggeramlug force-pushed the fix/5346-b33-hoist-precedence branch from 2354f8e to cdca560 Compare June 18, 2026 09:45
@proggeramlug proggeramlug merged commit c6875c3 into main Jun 18, 2026
15 checks passed
@proggeramlug proggeramlug deleted the fix/5346-b33-hoist-precedence branch June 18, 2026 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant