Skip to content

refactor(minifier): extract the class removability decision into a classifier#24386

Merged
graphite-app[bot] merged 1 commit into
mainfrom
minifier-class-removability-classifier
Jul 14, 2026
Merged

refactor(minifier): extract the class removability decision into a classifier#24386
graphite-app[bot] merged 1 commit into
mainfrom
minifier-class-removability-classifier

Conversation

@Dunqing

@Dunqing Dunqing commented Jul 11, 2026

Copy link
Copy Markdown
Member

remove_unused_class decides inline whether an unused class can be removed and what its removal leaves behind: side-effectful heritage or computed keys and present static values get extracted into the surrounding code. The next PR in this stack adds a liveness analysis that has to make the same decision at a different place and time, and the two decisions must agree exactly — a class the analysis treats as cleanly removable must not extract anything at the removal site, or the extracted expressions would reference declarations that were removed together with it.

This moves the decision into one classifier, classify_class_removability, which reports Keep, Extracts, or RemovesClean. remove_unused_class classifies once up front; RemovesClean skips the extraction walk it cannot need. The Extracts path still re-runs the heritage and computed-key purity checks in its extraction loop — the classifier is shared with callers that never extract, so it reports no extraction plan; that path only runs when an Extracts class is actually removed.

remove_unused_expression now also routes through expr_has_specialized_unused_handler, so the list of handlers that can leave residue cannot drift from the dispatch. The same analysis relies on that list to know which initializers are dropped whole. A new specialized arm added without a predicate entry becomes dead code that its author's own tests catch; a predicate entry without a dispatch arm hits an unreachable!.

No behavior change. Minified output and iteration counts are byte-identical on every fixture; 553 unit tests pass.

Implemented with AI assistance (Claude Code); design, review, and verification driven by the author per the repo AI-usage policy.

Dunqing commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

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

codspeed-hq Bot commented Jul 11, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 52 untouched benchmarks
⏩ 19 skipped benchmarks1


Comparing minifier-class-removability-classifier (cdc877f) with main (edf781d)

Open in CodSpeed

Footnotes

  1. 19 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 marked this pull request as ready for review July 11, 2026 14:39
@graphite-app
graphite-app Bot changed the base branch from minifier-class-heritage-throws to graphite-base/24386 July 11, 2026 14:43
@graphite-app
graphite-app Bot force-pushed the minifier-class-removability-classifier branch from 6893f78 to 0e84107 Compare July 11, 2026 14:48
@graphite-app
graphite-app Bot force-pushed the graphite-base/24386 branch from b97c045 to 7d33363 Compare July 11, 2026 14:48
@graphite-app
graphite-app Bot changed the base branch from graphite-base/24386 to main July 11, 2026 14:49
@graphite-app
graphite-app Bot force-pushed the minifier-class-removability-classifier branch from 0e84107 to 984ae8d Compare July 11, 2026 14:49
@Dunqing
Dunqing force-pushed the minifier-class-removability-classifier branch 5 times, most recently from 4e88e2b to cdc877f Compare July 14, 2026 00:51
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Jul 14, 2026

Dunqing commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

Merge activity

@Dunqing

Dunqing commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

Considering this is a pure refactor and makes the code cleaner, I will merge it first.

…assifier (#24386)

`remove_unused_class` decides inline whether an unused class can be removed and what its removal leaves behind: side-effectful heritage or computed keys and present static values get extracted into the surrounding code. The next PR in this stack adds a liveness analysis that has to make the same decision at a different place and time, and the two decisions must agree exactly — a class the analysis treats as cleanly removable must not extract anything at the removal site, or the extracted expressions would reference declarations that were removed together with it.

This moves the decision into one classifier, `classify_class_removability`, which reports `Keep`, `Extracts`, or `RemovesClean`. `remove_unused_class` classifies once up front; `RemovesClean` skips the extraction walk it cannot need. The `Extracts` path still re-runs the heritage and computed-key purity checks in its extraction loop — the classifier is shared with callers that never extract, so it reports no extraction plan; that path only runs when an `Extracts` class is actually removed.

`remove_unused_expression` now also routes through `expr_has_specialized_unused_handler`, so the list of handlers that can leave residue cannot drift from the dispatch. The same analysis relies on that list to know which initializers are dropped whole. A new specialized arm added without a predicate entry becomes dead code that its author's own tests catch; a predicate entry without a dispatch arm hits an `unreachable!`.

No behavior change. Minified output and iteration counts are byte-identical on every fixture; 553 unit tests pass.

Implemented with AI assistance (Claude Code); design, review, and verification driven by the author per the repo AI-usage policy.
@graphite-app
graphite-app Bot force-pushed the minifier-class-removability-classifier branch from cdc877f to c415c23 Compare July 14, 2026 13:59
@graphite-app
graphite-app Bot merged commit c415c23 into main Jul 14, 2026
29 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jul 14, 2026
@graphite-app
graphite-app Bot deleted the minifier-class-removability-classifier branch July 14, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant