-
-
Notifications
You must be signed in to change notification settings - Fork 163
perf(hir): class captures live in the class environment, not on instances #11297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
ae9177e
perf(hir): keep class captures in the class environment, not on insta…
a9f0656
perf(hir): drop class-environment rebinds a member never names
3a5ee30
test(hir): pin reflection hiding and per-evaluation capture before an…
9360557
fix(hir): run-once proof keys on the function's own arguments, not th…
0739059
perf(hir): guard the class environment by evaluation for fresh class …
a6c3004
chore(gc): re-pin the census window after the class-env scanner regis…
e2c0a40
fix(hir): a self-construction records its member's class evaluation
f1ba774
changelog: class captures live in the class environment
f9f295b
changelog: name the fragment after PR #11297
aa78b2e
fix(runtime): class environments keep the defining evaluation across …
dce6ca7
fix(hir): an env-mode refresh keeps the for-let expired-head rewrite
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| Classes nested in a function no longer store their captured outer variables | ||
| on every instance. A class whose definition runs once, or a class expression | ||
| evaluated to a fresh class object (every class in a CommonJS module body), | ||
| keeps its captures in a per-class environment read with one compare and one | ||
| load; a second evaluation (a re-run module body) is resolved per receiver, | ||
| so each instance still sees its own evaluation's values. TypeScript's AST | ||
| nodes lose their 3-10 hidden `__perry_cap_*` keys (25% fewer bytes per node), | ||
| `pos`/`end`/`kind` no longer shift with a class's capture count, and | ||
| `ts.transpileModule` runs about 11% fewer instructions. | ||
|
|
||
| A class expression in env mode that closes over a `for (let …)` head binding | ||
| keeps #11250's expired-head rewrite: the refresh re-reads that evaluation's | ||
| own capture array, which the runtime republishes into the environment slots | ||
| only for the owning evaluation (`test_gap_11297_env_class_for_let_capture`). | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the "resolved per receiver" claim.
Lines 5-6 say that a second evaluation "is resolved per receiver". The tests show different behavior. In
crates/perry/tests/class_capture_environment.rs,first.Box.prototype.get.call(b)returnse1even thoughbbelongs to the second evaluation (Lines 336 and 348).first.Box.prototype.make.call(b)also builds ane1instance (Lines 372 and 387). Capture resolution follows the evaluation of the member that runs. Instances record their evaluation for construction. The receiver does not decide the result. Change the text so it matches this behavior.📝 Proposed wording
📝 Committable suggestion
🤖 Prompt for AI Agents