fix(es): avoid incorrect instanceof optimizations - #12110
fix(es): avoid incorrect instanceof optimizations#12110Leo (teamleaderleo) wants to merge 6 commits into
instanceof optimizations#12110Conversation
🦋 Changeset detectedLatest commit: 9f838a5 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Updated the draft. |
Merging this PR will not alter performance
Comparing Footnotes
|
instanceof semantics during optimizationinstanceof evaluation
3724575 to
d9cc58e
Compare
d9cc58e to
0f2ad51
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9cc58ec36
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
9a7abca to
551c0d2
Compare
Preserve observable instanceof evaluation across effect extraction, simplification, minifier ignored-result handling, and dead-branch cleanup. Remove stale instanceof fold eligibility, add fixture-based regression coverage, and update the affected expectations.
551c0d2 to
a8ee1a6
Compare
|
Ok, so we keep the existing Terser assumptions for the unused |
instanceof evaluationinstanceof constant folding
|
I updated this PR. I kept the existing discarded-result behavior and reduced the PR to the incorrect |
| @@ -1,4 +1,4 @@ | |||
| foo() instanceof bar(); | |||
There was a problem hiding this comment.
Okay, my interpretation: keep the existing discarded instanceof behavior generally,
but leave the full expression intact when one of its operands comes from an expression SWC already marked pure.
I updated the PR that way.
There was a problem hiding this comment.
Sorry, I haven't seen foo has been marked as pure. foo() should be removed in this case.
The general idea is not to change any test case under tests/terser.
There was a problem hiding this comment.
Thank you for clarifying. I restored the tests/terser expectations and removed the pure-marked instanceof exception. The PR is back to only fixing the incorrect constant folding.
There was a problem hiding this comment.
Despite being useless, could you restore some harmless correct constant folding like 1 instanceof Object?
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1ffb993a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
instanceof constant foldinginstanceof optimizations
Description:
Closes #12111.
Avoid folding
instanceoffrom operand shape alone. The operator may invokeSymbol.hasInstance, and an invalid right-hand operand may throw, so folds such as object-like values totrueor primitive-like values tofalseare not generally valid.For example:
evaluates to
false, but could previously fold totrue.This keeps SWC's existing Terser-compatible treatment of discarded
instanceofexpressions unchanged. Tests undertests/terserare unchanged.The change removes the operand-shape-based
instanceoffolds in the minifier and expression simplifier and adds SWC-owned regressions for cases where the result is used.in/pure_gettersare unchanged.BREAKING CHANGE:
None.
Validation: