* SUMMARY
ES6: Classes: Should be allowed to create a static method with name "arguments".
jsc> class MyClass { static arguments() { } }
Exception: TypeError: Attempting to change configurable attribute of unconfigurable property.
* NOTES
- Behaves fine in Firefox and Chrome.
- See related bug 144281.
Comment on attachment 277870[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=277870&action=review> Source/JavaScriptCore/ChangeLog:10
> + class A { static argumetns() { return 'value'; } }
typo: 'argumetns' => 'arguments'
> Source/JavaScriptCore/runtime/JSFunction.cpp:368
> + if (!(thisObject->jsExecutable()->isClass() && slot.internalMethodType() == PropertySlot::InternalMethodType::GetOwnProperty)) {
Why do we only care to skip this if we're doing a GetOwnProperty access on a class?
We will not skip if we're doing a normal Get or VMInquiry or HasProperty?
> Source/JavaScriptCore/runtime/JSFunction.cpp:399
> + bool result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
> + if (!result) {
> + GetterSetter* errorGetterSetter = thisObject->jsExecutable()->isClassConstructorFunction() || thisObject->jsExecutable()->parseMode() == SourceParseMode::MethodMode
> + ? thisObject->globalObject()->throwTypeErrorArgumentsAndCallerGetterSetter(exec->vm())
> + : thisObject->globalObject()->throwTypeErrorGetterSetter(exec->vm());
> +
> + thisObject->putDirectAccessor(exec, propertyName, errorGetterSetter, DontDelete | DontEnum | Accessor);
> + result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
> + ASSERT(result);
> + }
> + return result;
Can we write a helper for the common code?
I see you are adding a new test. You should update the existing test that references this bug:
> LayoutTests/js/script-tests/keyword-method-names.js:79
> + // FIXME: <https://webkit.org/b/152985> ES6: Classes: Should be allowed to create a static method with name "arguments"
> + // static arguments() { }
Comment on attachment 278360[details]
Patch
I'm a little confused, if I have read the spec correctly (https://tc39.github.io/ecma262/#sec-forbidden-extensions), it looks like "arguments" and "caller" should not be on any non-strict function anyway. It seems like we should just get rid of those properties and make them act like normal for non-strict functions. I'm pretty sure that all class constructors are strict anyway so I think this would entirely fix this issue. Is there something I am missing?
(In reply to comment #10)
> Comment on attachment 278360[details]
> Patch
>
> I'm a little confused, if I have read the spec correctly
> (https://tc39.github.io/ecma262/#sec-forbidden-extensions), it looks like
> "arguments" and "caller" should not be on any non-strict function anyway. It
Hmm, I did not find that about non-strict function in this spec. Only about function in strict-mode and arrow function, class declaration and etc.
> seems like we should just get rid of those properties and make them act like
> normal for non-strict functions. I'm pretty sure that all class constructors
> are strict anyway so I think this would entirely fix this issue. Is there
> something I am missing?
Currently my patch does not cover arrow function, GeneratorDeclaration, GeneratorExpression and functions created using the bind. So I'll add try to cover those cases
Created attachment 279313[details]
Archive of layout-test-results from ews122 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4
(In reply to comment #11)
> (In reply to comment #10)
> > Comment on attachment 278360[details]
> > Patch
> >
> > I'm a little confused, if I have read the spec correctly
> > (https://tc39.github.io/ecma262/#sec-forbidden-extensions), it looks like
> > "arguments" and "caller" should not be on any non-strict function anyway. It
> Hmm, I did not find that about non-strict function in this spec. Only about
> function in strict-mode and arrow function, class declaration and etc.
> > seems like we should just get rid of those properties and make them act like
> > normal for non-strict functions. I'm pretty sure that all class constructors
> > are strict anyway so I think this would entirely fix this issue. Is there
> > something I am missing?
>
> Currently my patch does not cover arrow function, GeneratorDeclaration,
> GeneratorExpression and functions created using the bind. So I'll add try to
> cover those cases
I think you might have misunderstood my comment. I think that all strict functions should not have an "arguments" or "caller" property. Instead those properties should be on %FunctionPrototype% and should throw a type error on get or set.
For example,
function foo() {
"use strict";
return 1;
}
class bar {}
Object.getOwnPropertyDescriptor(foo, "arguments") // undefined
Object.getOwnPropertyDescriptor(bar, "arguments") // undefined
Object.getOwnPropertyDescriptor(foo, "caller") // undefined
Object.getOwnPropertyDescriptor(bar, "caller") // undefined
Object.getOwnPropertyDescriptor(foo.__proto__, "arguments") // { get: () => { throw new TypeError(), set: () => { throw new TypeError() }, writable: false, enumerable: false }
Object.getOwnPropertyDescriptor(foo.__proto__, "caller") // { get: () => { throw new TypeError(), set: () => { throw new TypeError() }, writable: false, enumerable: false }
Object.getOwnPropertyDescriptor(bar.__proto__, "arguments") // { get: () => { throw new TypeError(), set: () => { throw new TypeError() }, writable: false, enumerable: false }
Object.getOwnPropertyDescriptor(bar.__proto__, "caller") // { get: () => { throw new TypeError(), set: () => { throw new TypeError() }, writable: false, enumerable: false }
Does my example seem correct to you based on the spec?
(In reply to comment #17)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > Comment on attachment 278360[details]
> > > Patch
> > >
> > > I'm a little confused, if I have read the spec correctly
> > > (https://tc39.github.io/ecma262/#sec-forbidden-extensions), it looks like
> > > "arguments" and "caller" should not be on any non-strict function anyway. It
> > Hmm, I did not find that about non-strict function in this spec. Only about
> > function in strict-mode and arrow function, class declaration and etc.
> > > seems like we should just get rid of those properties and make them act like
> > > normal for non-strict functions. I'm pretty sure that all class constructors
> > > are strict anyway so I think this would entirely fix this issue. Is there
> > > something I am missing?
> >
> > Currently my patch does not cover arrow function, GeneratorDeclaration,
> > GeneratorExpression and functions created using the bind. So I'll add try to
> > cover those cases
>
> I think you might have misunderstood my comment. I think that all strict
> functions should not have an "arguments" or "caller" property. Instead those
> properties should be on %FunctionPrototype% and should throw a type error on
> get or set.
>
> For example,
> function foo() {
> "use strict";
> return 1;
> }
>
> class bar {}
>
>
> Object.getOwnPropertyDescriptor(foo, "arguments") // undefined
> Object.getOwnPropertyDescriptor(bar, "arguments") // undefined
> Object.getOwnPropertyDescriptor(foo, "caller") // undefined
> Object.getOwnPropertyDescriptor(bar, "caller") // undefined
>
> Object.getOwnPropertyDescriptor(foo.__proto__, "arguments") // { get: () =>
> { throw new TypeError(), set: () => { throw new TypeError() }, writable:
> false, enumerable: false }
> Object.getOwnPropertyDescriptor(foo.__proto__, "caller") // { get: () => {
> throw new TypeError(), set: () => { throw new TypeError() }, writable:
> false, enumerable: false }
>
> Object.getOwnPropertyDescriptor(bar.__proto__, "arguments") // { get: () =>
> { throw new TypeError(), set: () => { throw new TypeError() }, writable:
> false, enumerable: false }
> Object.getOwnPropertyDescriptor(bar.__proto__, "caller") // { get: () => {
> throw new TypeError(), set: () => { throw new TypeError() }, writable:
> false, enumerable: false }
>
> Does my example seem correct to you based on the spec?
I got it. I think you are right. It should be in this way.
Created attachment 281189[details]
Archive of layout-test-results from ews103 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 281190[details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Created attachment 281191[details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4
Created attachment 281193[details]
Archive of layout-test-results from ews117 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 281276[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=281276&action=review
So I'm a little confused what this patch changed. It would be great if the changelog went into more detail on what the code you changed does. It might also be helpful if you explained why you made the changes you did. Also, is there a reason why caller and arguments are not just normal static, lazy properties? Then you can have JSFunction intercept the lookup of arguments and caller for non-strict functions and it will probably make this code much simpler.
> Source/JavaScriptCore/runtime/FunctionPrototype.cpp:247
> + if (thisObject->inherits(JSFunction::info())) {
> + JSFunction* function = jsCast<JSFunction*>(thisObject);
You can just do if (JSFunction* function = jsDynamicCast<JSFunction*>(JSValue::decode(thisValue)))
> Source/JavaScriptCore/runtime/FunctionPrototype.cpp:254
> + ASSERT(!function->isHostFunction());
Is this code reachable?
(In reply to comment #31)
> Comment on attachment 281276[details]
> Patch
>
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281276&action=review
>
> So I'm a little confused what this patch changed. It would be great if the
> changelog went into more detail on what the code you changed does. It might
> also be helpful if you explained why you made the changes you did. Also, is
> there a reason why caller and arguments are not just normal static, lazy
> properties? Then you can have JSFunction intercept the lookup of arguments
> and caller for non-strict functions and it will probably make this code much
> simpler.
OK. I'll try explain my changes in more details in ChangeLog. I think mostly my patch is moving all logic that is related to caller/arguments from JSFunction to FunctionPrototype with small adjustments. The main reason why I moved caller/arguments from JSFunction to FunctionPrototype class, because I decided to have all code that is related to caller/arguments in one place:
So code that handle cases when lookup of arguments/caller of function and arguments/caller of function prototype in one place:
var foo = function () {};
var args = foo.arguments; // case 1
var proto = foo.__proto__;
var caller = proto.caller; // case 2
Do think it is better to split this logic between JSFunction and FunctionPrototype classes?
>
> > Source/JavaScriptCore/runtime/FunctionPrototype.cpp:247
> > + if (thisObject->inherits(JSFunction::info())) {
> > + JSFunction* function = jsCast<JSFunction*>(thisObject);
>
> You can just do if (JSFunction* function =
> jsDynamicCast<JSFunction*>(JSValue::decode(thisValue)))
>
> > Source/JavaScriptCore/runtime/FunctionPrototype.cpp:254
> > + ASSERT(!function->isHostFunction());
>
> Is this code reachable?
Created attachment 286917[details]
Archive of layout-test-results from ews101 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 286918[details]
Archive of layout-test-results from ews116 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 286920[details]
Archive of layout-test-results from ews121 for ios-simulator-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
Created attachment 286921[details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Created attachment 286955[details]
Archive of layout-test-results from ews101 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 286956[details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Created attachment 286957[details]
Archive of layout-test-results from ews116 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 286958[details]
Archive of layout-test-results from ews121 for ios-simulator-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
(In reply to comment #31)
> Comment on attachment 281276[details]
> Patch
>
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281276&action=review
>
> So I'm a little confused what this patch changed. It would be great if the
> changelog went into more detail on what the code you changed does. It might
> also be helpful if you explained why you made the changes you did. Also, is
> there a reason why caller and arguments are not just normal static, lazy
> properties? Then you can have JSFunction intercept the lookup of arguments
> and caller for non-strict functions and it will probably make this code much
> simpler.
>
I've made refactoring of the Patch as you suggested, so it allow to decrease size of changes
Created attachment 287017[details]
Archive of layout-test-results from ews102 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 287018[details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 286999[details]
Patch
Looks like EWS is angry that you deleted the class-method-and-constructor-properties.js file. Also, can you explain why "arguments" and "caller" are on Function prototype, I'm not sure why I follow that they need to be there?
Created attachment 287020[details]
Archive of layout-test-results from ews123 for ios-simulator-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
Created attachment 287042[details]
Archive of layout-test-results from ews112 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
(In reply to comment #59)
> Comment on attachment 286999[details]
> Patch
>
> Looks like EWS is angry that you deleted the
> class-method-and-constructor-properties.js file. Also, can you explain why
> "arguments" and "caller" are on Function prototype, I'm not sure why I
> follow that they need to be there?
I move this to the Function prototype to allow execute following code:
var f = function () {}
f.__proto__.caller; //Should raise type error
I've checked in Chrome and FireFox:
1) Chrome returns TypeError: 'caller' and 'arguments' are restricted function properties and cannot be accessed in this context
2) FireFox return just null.
Comment on attachment 287129[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=287129&action=review
I think this is getting close but I still think there are a few things that need to be changed.
> Source/JavaScriptCore/ChangeLog:62
> + * runtime/Executable.h:
> + * runtime/FunctionPrototype.cpp:
> + (JSC::FunctionPrototype::defineOwnProperty):
> + (JSC::FunctionPrototype::getThrowTypeErrorOwnPropertySlot):
> + (JSC::FunctionPrototype::getOwnPropertySlot):
> + (JSC::FunctionPrototype::getOwnNonIndexPropertyNames):
> + (JSC::functionProtoFuncBind): Deleted.
> + * runtime/FunctionPrototype.h:
> + * runtime/JSFunction.cpp:
> + (JSC::JSFunction::getOwnPropertySlot):
> + (JSC::JSFunction::getOwnNonIndexPropertyNames):
> + (JSC::JSFunction::put):
> + (JSC::JSFunction::deleteProperty):
> + (JSC::JSFunction::defineOwnProperty):
> + * runtime/JSFunction.h:
> + * runtime/JSObject.cpp:
> + (JSC::ordinarySetSlow):
> + (JSC::JSObject::getOwnPropertyDescriptor):
> + * runtime/PropertySlot.h:
> + (JSC::PropertySlot::PropertySlot):
> + (JSC::PropertySlot::setIsClassMethod):
> + (JSC::PropertySlot::isClassMethod):
> + (JSC::PropertySlot::isTaintedByOpaqueObject): Deleted.
Delete.
> Source/JavaScriptCore/runtime/FunctionPrototype.cpp:184
> + if (!object->inherits(JSFunction::info()))
> + return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
I don't think you need this check. object should always be FunctionPrototype*.
> Source/JavaScriptCore/runtime/FunctionPrototype.cpp:204
> + JSFunction* thisObject = jsCast<JSFunction*>(object);
> + if (thisObject->isHostOrBuiltinFunction())
> + return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
> +
> + if (propertyName == exec->propertyNames().arguments) {
> + if (thisObject->jsExecutable()->isStrictMode() || thisObject->jsExecutable()->isArrowFunction()) {
> + PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
> + if (!Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
> + thisObject->putDirectAccessor(exec, propertyName, thisObject->globalObject()->throwTypeErrorArgumentsCalleeAndCallerGetterSetter(), DontDelete | DontEnum | Accessor);
> + return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
> + }
> + } else if (propertyName == exec->propertyNames().caller) {
> + if (thisObject->jsExecutable()->isStrictMode() || thisObject->jsExecutable()->isArrowFunction()) {
> + PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
> + if (!Base::getOwnPropertySlot(thisObject, exec, propertyName, slot))
> + thisObject->putDirectAccessor(exec, propertyName, thisObject->globalObject()->throwTypeErrorArgumentsCalleeAndCallerGetterSetter(), DontDelete | DontEnum | Accessor);
> + return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
> + }
> + }
I think this could just be FunctionPrototype* thisObject = jsCast<FunctionPrototype*>(object);
You would need to add a reifyLazyPropertyIfNeeded function to FunctionPrototype that handles arguments and length like:
void FunctionPrototype::reifyLazyPropertyIfNeeded(exec, propertyName) {
if (propertyName == exec->propertyNames().arguments || propertyName == exec->propertyNames().caller)
// Do reification of arguments/caller to type error getter/setter if needed.
Base::reifyLazyPropertyIfNeeded(exec, propertyName);
}
> Source/JavaScriptCore/runtime/FunctionPrototype.cpp:231
> + if (propertyName == exec->propertyNames().caller || propertyName == exec->propertyNames().arguments) {
> + // For non-host functions, don't let these properties by deleted - except by DefineOwnProperty.
> + if (slot.thisValue().inherits(JSFunction::info())) {
> + JSFunction* thisObject = jsDynamicCast<JSFunction*>(slot.thisValue());
> + if (!thisObject->isHostOrBuiltinFunction())
> + return getThrowTypeErrorOwnPropertySlot(object, exec, propertyName, slot);
> + } else if (slot.thisValue().inherits(FunctionPrototype::info()))
> + return getThrowTypeErrorOwnPropertySlot(object, exec, propertyName, slot);
> + }
This could just be (if you do my change above):
FunctionPrototype* thisObject = jsCast<FunctionPrototype*>(object);
thisObject->reifyLazyPropertyIfNeeded(exec, propertyName);
> Source/JavaScriptCore/runtime/FunctionPrototype.cpp:246
> + VM& vm = exec->vm();
> + // Make sure prototype has been reified.
> + PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
> + thisObject->methodTable(vm)->getOwnPropertySlot(thisObject, exec, vm.propertyNames->prototype, slot);
> +
> + propertyNames.add(vm.propertyNames->arguments);
> + propertyNames.add(vm.propertyNames->caller);
Ditto.
> Source/JavaScriptCore/runtime/JSFunction.cpp:402
> + if (!thisObject->jsExecutable()->isStrictMode()) {
> + propertyNames.add(vm.propertyNames->arguments);
> + propertyNames.add(vm.propertyNames->caller);
> + }
You might need to change this to work with my proposal for reification.
> Source/JavaScriptCore/runtime/JSObject.cpp:517
> + // We do not update the value of caller & arguments properties in function
Why do we not allow the user to set arguments on a strict function?
> Source/JavaScriptCore/runtime/JSObject.cpp:518
> + if (object->inherits(JSFunction::info()))
I think we usually just do this with jsDynamicCast<JSFunction>(object)
> Source/JavaScriptCore/runtime/JSObject.cpp:2858
> + if (propertyName == exec->propertyNames().arguments || propertyName == exec->propertyNames().caller) {
> + if (auto thisObject = jsDynamicCast<JSFunction*>(this)) {
> + if (!thisObject->isHostFunctionNonInline() && thisObject->jsExecutable()->isClass() && !thisObject->jsExecutable()->isClassConstructorFunction())
> + slot.setIsClassMethod();
> + }
> + }
What does this do? It doesn't look like you use the isClassMethod bit anywhere?
Comment on attachment 287129[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=287129&action=review>> Source/JavaScriptCore/ChangeLog:62
>> + (JSC::PropertySlot::isTaintedByOpaqueObject): Deleted.
>
> Delete.
Done
>> Source/JavaScriptCore/runtime/FunctionPrototype.cpp:184
>> + return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
>
> I don't think you need this check. object should always be FunctionPrototype*.
Remove whole method :-)
>> Source/JavaScriptCore/runtime/FunctionPrototype.cpp:204
>> + }
>
> I think this could just be FunctionPrototype* thisObject = jsCast<FunctionPrototype*>(object);
>
> You would need to add a reifyLazyPropertyIfNeeded function to FunctionPrototype that handles arguments and length like:
>
> void FunctionPrototype::reifyLazyPropertyIfNeeded(exec, propertyName) {
> if (propertyName == exec->propertyNames().arguments || propertyName == exec->propertyNames().caller)
> // Do reification of arguments/caller to type error getter/setter if needed.
> Base::reifyLazyPropertyIfNeeded(exec, propertyName);
> }
Removed whole method
>> Source/JavaScriptCore/runtime/FunctionPrototype.cpp:231
>> + }
>
> This could just be (if you do my change above):
>
> FunctionPrototype* thisObject = jsCast<FunctionPrototype*>(object);
> thisObject->reifyLazyPropertyIfNeeded(exec, propertyName);
Removed whole method
>> Source/JavaScriptCore/runtime/FunctionPrototype.cpp:246
>> + propertyNames.add(vm.propertyNames->caller);
>
> Ditto.
Removed whole method
>> Source/JavaScriptCore/runtime/JSFunction.cpp:402
>> + }
>
> You might need to change this to work with my proposal for reification.
I've just added small check, to add this method for function not in Strict mode
>> Source/JavaScriptCore/runtime/JSObject.cpp:517
>> + // We do not update the value of caller & arguments properties in function
>
> Why do we not allow the user to set arguments on a strict function?
Removed whole change
>> Source/JavaScriptCore/runtime/JSObject.cpp:2858
>> + }
>
> What does this do? It doesn't look like you use the isClassMethod bit anywhere?
Yes, you are right. It is not used anymore. Removed
Comment on attachment 287871[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=287871&action=review> Source/JavaScriptCore/runtime/JSFunction.cpp:404
> + if (!thisObject->jsExecutable()->isStrictMode()) {
Don't you want !thisObject->jsExecutable()->isStrictMode() && !thisObject->jsExecutable()->isES6Function()?
Also, does this work with Function.prototype when the arguments property has been deleted? i.e.
delete Function.prototype.arguments;
for (name in Function.prototype) {
if (name !== "arguments")
throw new Error("bad name");
}
Comment on attachment 287871[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=287871&action=review>> Source/JavaScriptCore/runtime/JSFunction.cpp:404
>> + if (!thisObject->jsExecutable()->isStrictMode()) {
>
> Don't you want !thisObject->jsExecutable()->isStrictMode() && !thisObject->jsExecutable()->isES6Function()?
>
> Also, does this work with Function.prototype when the arguments property has been deleted? i.e.
>
> delete Function.prototype.arguments;
> for (name in Function.prototype) {
> if (name !== "arguments")
> throw new Error("bad name");
> }
Yeah, I've add !thisObject->jsExecutable()->isES6Function() condition to the new patch.
In case of 'delete Function.prototype.arguments;' we delete it from FunctionPrototyp(FunctionPrototype.cpp) where it added with property DontEnum, so it will not appear during for..in operation before and after delete operation.
(In reply to comment #73)
> Comment on attachment 287871[details]
> Patch
>
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=287871&action=review
>
> >> Source/JavaScriptCore/runtime/JSFunction.cpp:404
> >> + if (!thisObject->jsExecutable()->isStrictMode()) {
> >
> > Don't you want !thisObject->jsExecutable()->isStrictMode() && !thisObject->jsExecutable()->isES6Function()?
> >
> > Also, does this work with Function.prototype when the arguments property has been deleted? i.e.
> >
> > delete Function.prototype.arguments;
> > for (name in Function.prototype) {
> > if (name !== "arguments")
> > throw new Error("bad name");
> > }
>
> Yeah, I've add !thisObject->jsExecutable()->isES6Function() condition to the
> new patch.
>
> In case of 'delete Function.prototype.arguments;' we delete it from
> FunctionPrototyp(FunctionPrototype.cpp) where it added with property
> DontEnum, so it will not appear during for..in operation before and after
> delete operation.
Ah, I was thinking that FunctionPrototype subclassed JSFunction but I see that it actually subclasses InternalFunction. Then, you're right, it should work for Function.prototype.
Created attachment 288469[details]
Archive of layout-test-results from ews101 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 288471[details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Created attachment 288474[details]
Archive of layout-test-results from ews115 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 288480[details]
Archive of layout-test-results from ews125 for ios-simulator-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
Comment on attachment 288483[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=288483&action=review
r=me if you fix the typo. Thanks, for powering through this patch! :D
> Source/JavaScriptCore/ChangeLog:15
> + To implement this patch âcallerâ and âargumentsâ were put to the FunctionPrototype
should be "caller" and "arguments".
2016-05-01 15:09 PDT, GSkachkov
2016-05-02 06:05 PDT, GSkachkov
2016-05-07 16:25 PDT, GSkachkov
2016-05-08 00:19 PDT, GSkachkov
2016-05-08 04:18 PDT, GSkachkov
2016-05-18 14:59 PDT, GSkachkov
2016-05-18 16:23 PDT, Build Bot
2016-05-23 12:58 PDT, GSkachkov
2016-05-25 13:02 PDT, GSkachkov
2016-06-13 11:44 PDT, GSkachkov
2016-06-13 12:01 PDT, GSkachkov
2016-06-13 12:56 PDT, Build Bot
2016-06-13 12:58 PDT, Build Bot
2016-06-13 13:09 PDT, Build Bot
2016-06-13 13:12 PDT, Build Bot
2016-06-14 13:25 PDT, GSkachkov
2016-06-14 13:42 PDT, GSkachkov
2016-08-24 14:30 PDT, GSkachkov
2016-08-24 16:17 PDT, GSkachkov
2016-08-24 17:20 PDT, Build Bot
2016-08-24 17:27 PDT, Build Bot
2016-08-24 17:29 PDT, Build Bot
2016-08-24 17:35 PDT, Build Bot
2016-08-25 02:09 PDT, GSkachkov
2016-08-25 03:10 PDT, Build Bot
2016-08-25 03:15 PDT, Build Bot
2016-08-25 03:18 PDT, Build Bot
2016-08-25 03:27 PDT, Build Bot
2016-08-25 13:18 PDT, GSkachkov
2016-08-25 14:21 PDT, Build Bot
2016-08-25 14:25 PDT, Build Bot
2016-08-25 14:35 PDT, Build Bot
2016-08-25 16:32 PDT, Build Bot
2016-08-26 12:26 PDT, GSkachkov
2016-09-03 15:42 PDT, GSkachkov
2016-09-09 16:26 PDT, GSkachkov
2016-09-09 17:31 PDT, GSkachkov
2016-09-09 18:38 PDT, Build Bot
2016-09-09 18:42 PDT, Build Bot
2016-09-09 18:53 PDT, Build Bot
2016-09-09 20:53 PDT, Build Bot
2016-09-10 00:07 PDT, GSkachkov