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][optim] Add map and set constructors #32697

Merged
merged 1 commit into from
Mar 24, 2025
Merged

[compiler][optim] Add map and set constructors #32697

merged 1 commit into from
Mar 24, 2025

Conversation

mofeiZ
Copy link
Contributor

@mofeiZ mofeiZ commented Mar 20, 2025

  • Adds isConstructor: boolean to FunctionType. With this PR, each typed function can either be a constructor (currently only known globals) or non constructor. Alternatively, we prefer to encode polymorphic types / effects (and match the closest subtype)

  • Add Map and Set globals + built-ins


Stack created with Sapling. Best reviewed with ReviewStack.

@josephsavona
Copy link
Contributor

Yesssssss

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.

Repeating my comment: yessss i've wanted to have typing/effects for constructors and Map/Set for a long time. So glad you're adding this. A few small comments but overall looks great

[
'has',
addFunction(BUILTIN_SHAPES, [], {
positionalParams: [],
Copy link
Contributor

Choose a reason for hiding this comment

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

has() takes an argument

* forEach(callbackFn)
* forEach(callbackFn, thisArg)
*/
'forEach',
Copy link
Contributor

Choose a reason for hiding this comment

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

this mutates only if the arguments are mutable, right?

}),
],
/**
* Iteraturs
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: spelling

],
['size', PRIMITIVE_TYPE],
[
'forEach',
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@@ -506,8 +521,10 @@ class Unifier {
}

if (tB.kind === 'Function' && tA.kind === 'Function') {
this.unify(tA.return, tB.return);
return;
if (tA.isConstructor === tB.isConstructor) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could collapse these ifs

mofeiZ added a commit that referenced this pull request Mar 24, 2025
…perands are globals (#32695)

Globals, module locals, and other locally defined functions may mutate
their arguments. See test fixtures for details
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32695).
* #32698
* #32697
* #32696
* __->__ #32695
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
* Adds `isConstructor: boolean` to `FunctionType`. With this PR, each typed function can either be a constructor (currently only known globals) or non constructor. Alternatively, we prefer to encode polymorphic types / effects (and match the closest subtype)

* Add Map and Set globals + built-ins
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)
@mofeiZ mofeiZ merged commit a8e503d into main Mar 24, 2025
23 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 24, 2025
* Adds `isConstructor: boolean` to `FunctionType`. With this PR, each
typed function can either be a constructor (currently only known
globals) or non constructor. Alternatively, we prefer to encode
polymorphic types / effects (and match the closest subtype)

* Add Map and Set globals + built-ins
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32697).
* #32698
* __->__ #32697

DiffTrain build for [a8e503d](a8e503d)
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