feat(mangler): skip mangling only in scopes affected by direct eval#17612
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
camc314
force-pushed
the
c/01-03-fix_mangler_skip_mangling_only_in_scopes_affected_by_direct_eval
branch
from
393e12e to
81d681d
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes the mangler to only skip mangling in scopes that are actually affected by direct eval, rather than disabling all mangling globally when any direct eval is detected. The change improves code minification by allowing mangling in scopes that are isolated from eval's reach.
Key changes:
- Removed the early return that globally disabled mangling when direct eval was found anywhere in the code
- Added per-scope checks to skip mangling only in scopes that contain direct eval or have it in descendant scopes
- Added comprehensive test coverage for various direct eval scenarios including nested scopes, sibling scopes, child function scopes, and indirect eval
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/oxc_mangler/src/lib.rs | Removed global direct eval check and added per-scope checks during slot assignment and frequency tallying to selectively skip mangling |
| crates/oxc_minifier/tests/mangler/mod.rs | Added comprehensive test cases validating correct behavior for direct eval in various scope configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CodSpeed Performance ReportMerging #17612 will not alter performanceComparing Summary
Footnotes
|
|
It would be helpful if we have a comment here why we don't have to add var names in direct eval contained scopes. oxc/crates/oxc_mangler/src/lib.rs Line 458 in 81d681d |
camc314
force-pushed
the
c/01-03-fix_mangler_skip_mangling_only_in_scopes_affected_by_direct_eval
branch
from
81d681d to
82fb107
Compare
sapphi-red
changed the title
Merge activity
|
graphite-app
bot
force-pushed
the
c/01-03-fix_mangler_skip_mangling_only_in_scopes_affected_by_direct_eval
branch
from
82fb107 to
23680a3
Compare
graphite-app
bot
deleted the
c/01-03-fix_mangler_skip_mangling_only_in_scopes_affected_by_direct_eval
branch
### 🚀 Features - 659c23e linter: Init note field boilerplate (#17589) (Shrey Sudhir) - 6870b64 parser: Add TS1363 error code (#17609) (Sysix) - 23680a3 mangler: Skip mangling only in scopes affected by direct eval (#17612) (camc314) - a7e1643 parser: Add TS2528 error code to duplicate_default_export diagnostic (#17558) (camc314) ### 🐛 Bug Fixes - 1044116 ecmascript: Mark `new Symbol` as non side-effect free (#17568) (camc314) - ab5e4ca isolated-declarations: Strip default values from rest parameter binding patterns (#17602) (camc314) - 68b2e54 minifier: Prevent incorrect ??= transformation when member base is mutated (#17472) (copilot-swe-agent) ### ⚡ Performance - 6067143 semantic: Remove hash when checking identifier (#17564) (camchenry) - a28ab3d semantic: Avoid bounds check when checking string literal (#17545) (camc314) - 04809d1 semantic: Use SIMD for finding backslashes in `check_string_literal` (#17534) (camchenry) - 49ad2f0 semantic: Mark all diagnostic functions as `#[cold]` (#17487) (camc314) - ea82b50 transformer: Mark all diagnostic functions as `#[cold]` (#17486) (camc314) - d968e51 semantic: Mark `checker::check` as `inline(always)` (#17459) (camc314)
No description provided.