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
Add remaining private methods #46
base: main
Are you sure you want to change the base?
Conversation
macro($unscopables) | ||
#else | ||
#define BUN_JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(macro) | ||
#endif |
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.
Conditionally add private names if USE(BUN_JSC_ADDITIONS).
577ceea
to
842bdf0
Compare
842bdf0
to
b8b7097
Compare
1f396a2
to
a22b5f4
Compare
@@ -210,7 +211,7 @@ namespace JSC { | |||
macro(hasOwn) \ | |||
macro(indexOf) \ | |||
macro(pop) \ | |||
macro(asyncContext) \ | |||
|
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.
☝️ extra newline aligns with WebKit repo source (so closer to original source material).
#if USE(BUN_JSC_ADDITIONS) | ||
ExpressionNode* expr = n->isArgumentList() ? static_cast<ArgumentListNode*>(n)->m_expr : nullptr; | ||
if (expr && expr->isLinkTimeConstant()) return moveLinkTimeConstant(newTemporary(), static_cast<BytecodeIntrinsicNode*>(expr)->linkTimeConstant()); | ||
#endif |
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.
☝️ With this tweak in emitNode
we now can pass byteCodeIntrinsics and linkTimeConstants as arguments 😮
@@ -33,6 +33,7 @@ class CodeBlock; | |||
class JSGlobalObject; | |||
|
|||
#define JSC_FOREACH_LINK_TIME_CONSTANTS(v) \ | |||
BUN_JSC_LINK_TIME_CONSTANTS(v) \ |
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 lil' macro lives in CommonIdentifiers.h and co-locates all of our BUN_JSC_ADDITIONS names
my $key = $1; | ||
my $val = $2; | ||
my $att = $3; | ||
my $param = $4; | ||
my $intrinsic = $5; | ||
my $if = $6; |
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.
We now support conditionally reifying of props (with something like #if(USE(BUN_JSC_ADDITIONS))
) as seen here. Example:
/* Source for JSGlobalObject.lut.h
@begin globalObjectTable
isNaN globalFuncIsNaN DontEnum|Function 1 GlobalIsNaNIntrinsic
isFinite JSBuiltin DontEnum|Function 1
escape globalFuncEscape DontEnum|Function 1
SuppressedError JSGlobalObject::m_foo DontEnum|ClassStructure #if(USE(BUN_JSC_ADDITIONS))
Proxy createProxyProperty DontEnum|PropertyCallback
@end
*/
#if USE(BUN_JSC_ADDITIONS) | ||
if (ident.isPrivateName() || ident.isWellKnownSymbol(vm)) | ||
return identifierToJSValue(vm, ident); | ||
#endif |
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.
☝️ Allows passing @@iterator
and other symbols to methods like foo.@__lookupGetter(@@iterator)
.
#define FOR_EACH_JSC_COMMON_ISO_SUBSPACE(v) \ | ||
BUN_JSC_COMMON_ISO_SUBSPACES(v) \ |
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.
☝️ Small injection point for Bun addition names
#if USE(BUN_JSC_ADDITIONS) | ||
if (ident.isPrivateName() || ident.isWellKnownSymbol(vm)) | ||
return identifierToJSValue(vm, ident); | ||
#endif |
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.
#if USE(BUN_JSC_ADDITIONS) | ||
if (ident.isPrivateName() || ident.isWellKnownSymbol(vm)) | ||
return identifierToJSValue(vm, ident); | ||
#endif |
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.
#if USE(BUN_JSC_ADDITIONS) | ||
virtual bool isArgumentList() const { return false; } | ||
virtual bool isLinkTimeConstant() const { return false; } | ||
#endif |
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.
☝️ Make working with these Nodes easier (less casting between types).
@@ -950,6 +954,9 @@ namespace JSC { | |||
|
|||
ArgumentListNode* m_next { nullptr }; | |||
ExpressionNode* m_expr; | |||
#if USE(BUN_JSC_ADDITIONS) | |||
bool isArgumentList() const final { return true; } | |||
#endif |
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.
☝️ Allows us to detect the node in emitNode
macro(subarray) \ | ||
/* WeakRef */ \ | ||
macro(deref) \ | ||
|
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.
☝️ All the names together, shared, and organized 💆
vm.propertyNames->builtinNames().lookUpWellKnownSymbol(uid->substring(strlen("Symbol."))) != nullptr | ||
); | ||
} | ||
#endif |
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.
Makes working with this https://github.com/oven-sh/WebKit/pull/46/files#r1544482562 easier.
fields->putInternalField(vm, 0, a); | ||
fields->putInternalField(vm, 1, b); | ||
fields->finishCreation(vm); |
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.
Minor nit. Reuse in a way to longer adding fields After finishCreation
.
|
||
globalObject->queueMicrotask(job, argument0, argument1, argument2, argument3, argument4); |
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.
Support passing a 4th extra argument to queueMicrotask
to avoid jamming everything into the 3rd (required duck typing and extra checks to manage).
@@ -28,7 +28,6 @@ | |||
#include "ArrayAllocationProfile.h" | |||
#include "ArrayConstructor.h" | |||
#include "ArrayPrototype.h" | |||
#include "ErrorType.h" |
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.
While the inclusion makes sense the original WebKit source omits this so I am too (closer to original source).
a22b5f4
to
87c3d24
Compare
87c3d24
to
8dcfec5
Compare
Superseded by the jdalton/webkit-bun-patch branch. |
Add remaining private methods.
Related to oven-sh/bun#9306 and oven-sh/bun#9076
Example
Output