[webkit-reviews] review denied: [Bug 230602] [JSC] implement Shadow Realm : [Attachment 440773] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has denied Phillip Mates
<pmates at igalia.com>'s request for review:
Bug 230602: [JSC] implement Shadow Realm
https://bugs.webkit.org/show_bug.cgi?id=230602

Attachment 440773: Patch

https://bugs.webkit.org/attachment.cgi?id=440773&action=review




--- Comment #19 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 440773
  --> https://bugs.webkit.org/attachment.cgi?id=440773
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440773&action=review

commented

> Source/JavaScriptCore/builtins/ShadowRealmPrototype.js:33
> +	       var wrappedArgs = args. at map(@wrap)

Please check the comment in https://bugs.webkit.org/show_bug.cgi?id=230602#c16
:)

> Source/JavaScriptCore/bytecode/LinkTimeConstant.h:66
> +    v(ShadowRealm, nullptr) \

We should remove this since it is not used. It must be defined only when
@ShadowRealm is used in builtin JS.

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:108
> +   
JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->builtinNames().mapPri
vateName(), arrayPrototypeMapCodeGenerator,
static_cast<unsigned>(PropertyAttribute::DontEnum));

I mean, we should not use @map and remove this. See comment in
https://bugs.webkit.org/show_bug.cgi?id=230602#c16

> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:1042
> +   
m_linkTimeConstants[static_cast<unsigned>(LinkTimeConstant::ShadowRealm)].set(v
m, this, shadowRealmConstructor);

You should remove this too.