-
Notifications
You must be signed in to change notification settings - Fork 47k
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][ez] Add more Array.prototype methods #30075
Conversation
[ghstack-poisoned]
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
positionalParams: [], | ||
restParam: null, | ||
returnType: { kind: "Poly" }, | ||
calleeEffect: Effect.Store, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to push
, we know that pop
does not perform interior mutability. Effect.Store
here models pop similar to const arr = []; /*...*/; arr[2] = undefined;
(writing exclusively to the container, not to any internal elements)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clever!
@@ -478,6 +521,90 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [ | |||
noAlias: true, | |||
}), | |||
], | |||
[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pasted these to BuiltInMixedReadonlyId
, which is our read-only JSON-like object kind.
Happy to remove these and refactor later if we prefer to not have duplicates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is great!
Comparing: 133ada7...56d29f2 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yesssss thank you
positionalParams: [], | ||
restParam: null, | ||
returnType: { kind: "Poly" }, | ||
calleeEffect: Effect.Store, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clever!
@@ -478,6 +521,90 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [ | |||
noAlias: true, | |||
}), | |||
], | |||
[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is great!
Adds Array.prototype methods that return primitives or other arrays -- naive type inference can be really helpful in reducing mutable ranges -> achieving higher quality memoization. Also copies Array.prototype methods to our mixed read-only JSON-like object shape. (Inspired after going through some suboptimal internal compilation outputs.) [ghstack-poisoned]
Adds Array.prototype methods that return primitives or other arrays -- naive type inference can be really helpful in reducing mutable ranges -> achieving higher quality memoization. Also copies Array.prototype methods to our mixed read-only JSON-like object shape. (Inspired after going through some suboptimal internal compilation outputs.) ghstack-source-id: 0bfad11180992fc5db61a1c5f23954f48acf07b8 Pull Request resolved: #30075
Stack from ghstack (oldest at bottom):
Adds Array.prototype methods that return primitives or other arrays -- naive type inference can be really helpful in reducing mutable ranges -> achieving higher quality memoization.
Also copies Array.prototype methods to our mixed read-only JSON-like object shape.
(Inspired after going through some suboptimal internal compilation outputs.)