Skip to content

refactor(ecmascript): consolidate side-effect known globals into separate module#20401

Merged
graphite-app[bot] merged 1 commit intomainfrom
refactor/side-effects-consolidate-helpers
Mar 17, 2026
Merged

refactor(ecmascript): consolidate side-effect known globals into separate module#20401
graphite-app[bot] merged 1 commit intomainfrom
refactor/side-effects-consolidate-helpers

Conversation

@Dunqing
Copy link
Member

@Dunqing Dunqing commented Mar 15, 2026

Summary

  • Extract all global lookup tables (~430 lines) into a new known_globals.rs module, keeping expressions.rs focused on MayHaveSideEffects trait implementations
  • Extract is_pure_math_method to deduplicate 36 Math method names shared between read-safety (is_known_global_property) and call-safety (is_pure_global_method_call) checks
  • Extract inline call-safe match into a named is_pure_global_method_call function with clear doc comments
  • Rename is_pure_callis_pure_callable_constructor, removing 4 dead entries (String, Number, BigInt, Symbol) already handled by earlier special-case logic
  • Redefine is_unconditionally_pure_constructor in terms of is_pure_callable_constructor to keep error-type lists in sync
  • Fix: use is_typed_array_constructor for the TypedArray .of() arm, adding previously missing BigInt64Array and BigUint64Array

Test plan

  • Added tests for BigInt64Array.of() and BigUint64Array.of()
  • All existing side_effects tests pass
  • cargo clippy clean

🤖 Generated with Claude Code

@github-actions github-actions bot added A-minifier Area - Minifier C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Mar 15, 2026
Copy link
Member Author

Dunqing commented Mar 15, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of 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.

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 15, 2026

Merging this PR will not alter performance

✅ 53 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing refactor/side-effects-consolidate-helpers (9e63e4a) with fix/new-expression-argument-validation (ada6427)

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Dunqing Dunqing force-pushed the refactor/side-effects-consolidate-helpers branch from 3c4a9ec to f205721 Compare March 15, 2026 13:58
@Dunqing Dunqing changed the title refactor(ecmascript): consolidate side-effect detection helper functions refactor(ecmascript): consolidate side-effect known globals into separate module Mar 15, 2026
@Dunqing Dunqing force-pushed the fix/new-expression-argument-validation branch from 305e281 to ada6427 Compare March 15, 2026 14:03
@Dunqing Dunqing force-pushed the refactor/side-effects-consolidate-helpers branch from 3c200c3 to 9e63e4a Compare March 15, 2026 14:03
@Dunqing Dunqing marked this pull request as ready for review March 15, 2026 14:25
@Dunqing Dunqing requested review from Boshen and sapphi-red March 15, 2026 14:25
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Mar 15, 2026
Copy link
Member

Boshen commented Mar 15, 2026

Merge activity

@graphite-app graphite-app bot force-pushed the fix/new-expression-argument-validation branch 2 times, most recently from 5ebee41 to b03fc1d Compare March 16, 2026 09:03
@graphite-app graphite-app bot force-pushed the refactor/side-effects-consolidate-helpers branch from 9e63e4a to ded6e3a Compare March 16, 2026 09:03
@Dunqing Dunqing force-pushed the fix/new-expression-argument-validation branch from b03fc1d to 7c0f44d Compare March 16, 2026 15:00
@Dunqing Dunqing force-pushed the refactor/side-effects-consolidate-helpers branch from ded6e3a to 0fb31df Compare March 16, 2026 15:00
@graphite-app graphite-app bot changed the base branch from fix/new-expression-argument-validation to graphite-base/20401 March 17, 2026 14:53
@graphite-app graphite-app bot force-pushed the graphite-base/20401 branch from 7c0f44d to efeba28 Compare March 17, 2026 15:06
@graphite-app graphite-app bot force-pushed the refactor/side-effects-consolidate-helpers branch from 0fb31df to e67ca72 Compare March 17, 2026 15:06
@graphite-app graphite-app bot changed the base branch from graphite-base/20401 to main March 17, 2026 15:06
@graphite-app graphite-app bot force-pushed the refactor/side-effects-consolidate-helpers branch from e67ca72 to 0922623 Compare March 17, 2026 15:06
…rate module (#20401)

## Summary

- Extract all global lookup tables (~430 lines) into a new `known_globals.rs` module, keeping `expressions.rs` focused on `MayHaveSideEffects` trait implementations
- Extract `is_pure_math_method` to deduplicate 36 Math method names shared between read-safety (`is_known_global_property`) and call-safety (`is_pure_global_method_call`) checks
- Extract inline call-safe match into a named `is_pure_global_method_call` function with clear doc comments
- Rename `is_pure_call` → `is_pure_callable_constructor`, removing 4 dead entries (`String`, `Number`, `BigInt`, `Symbol`) already handled by earlier special-case logic
- Redefine `is_unconditionally_pure_constructor` in terms of `is_pure_callable_constructor` to keep error-type lists in sync
- **Fix**: use `is_typed_array_constructor` for the TypedArray `.of()` arm, adding previously missing `BigInt64Array` and `BigUint64Array`

## Test plan

- [x] Added tests for `BigInt64Array.of()` and `BigUint64Array.of()`
- [x] All existing `side_effects` tests pass
- [x] `cargo clippy` clean

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app bot force-pushed the refactor/side-effects-consolidate-helpers branch from 0922623 to 878eace Compare March 17, 2026 15:44
@graphite-app graphite-app bot merged commit 878eace into main Mar 17, 2026
21 checks passed
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 17, 2026
@graphite-app graphite-app bot deleted the refactor/side-effects-consolidate-helpers branch March 17, 2026 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants