Skip to content
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

[compiler][fix] mutableOnlyIfOperandsAreMutable does not apply when operands are globals #32695

Merged
merged 1 commit into from
Mar 24, 2025

Conversation

mofeiZ
Copy link
Contributor

@mofeiZ mofeiZ commented Mar 20, 2025

…perands are globals

Globals, module locals, and other locally defined functions may mutate their arguments. See test fixtures for details
@@ -101,6 +90,23 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [
returnValueKind: ValueKind.Mutable,
}),
],
[
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this definition for unit test (see newly added repro-array-map-known-mutate-shape.tsx). It was hard finding a global function that (1) has no mutable receiver, (2) only takes one argument, and (3) mutates its argument.

@@ -372,6 +378,86 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [
returnValueKind: ValueKind.Primitive,
}),
],
[
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added these drive-by (similar to Boolean) to try to limit scope of deopt.

Copy link
Contributor

@josephsavona josephsavona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, very subtle. Nice find/fix!

@mofeiZ mofeiZ merged commit febc09b into main Mar 24, 2025
25 checks passed
mofeiZ added a commit that referenced this pull request Mar 24, 2025
…handling (#32696)

Simplify InferReferenceEffect function signature matching logic for next
PRs in stack
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32696).
* #32698
* #32697
* __->__ #32696
* #32695
github-actions bot pushed a commit that referenced this pull request Mar 24, 2025
…handling (#32696)

Simplify InferReferenceEffect function signature matching logic for next
PRs in stack
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32696).
* #32698
* #32697
* __->__ #32696
* #32695

DiffTrain build for [45463ab](45463ab)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants