[Webkit-unassigned] [Bug 159398] [test262] Fixing mapped arguments object property test case
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Oct 9 01:08:30 PDT 2016
https://bugs.webkit.org/show_bug.cgi?id=159398
--- Comment #70 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 290583
--> https://bugs.webkit.org/attachment.cgi?id=290583
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=290583&action=review
> Source/JavaScriptCore/ChangeLog:11
> + property. Also it is fixing cases where arguments[i]
> + cannot be deleted when argument "i" is {configurable: false}.
I think you mean you're making it so that arguments[i] can't be deleted, correct? I would maybe write "fixing cases where" => "ensuring that"
> Source/JavaScriptCore/ChangeLog:13
> + The current implementation is against to the specification for 2 reasons:
I would say "previous" instead of "current" if you're referring to the behavior that is ToT now and what will be the old behavior as your patch lands.
> Source/JavaScriptCore/ChangeLog:18
> + It means that we just stop aliasing an property index already
> + defined if its property descriptor contains writable and its value
I'm confused by what you mean here.
> Source/JavaScriptCore/ChangeLog:22
> + 2. When a property is overriden it is always returning true. However
I would say "delete" instead of "it"
> Source/JavaScriptCore/runtime/DirectArguments.cpp:90
> + size_t configurablesSize = thisObject->m_configurableMap ? thisObject->m_length : 0;
Please make this * sizeof(bool). I would also argue that the overridesSize variable above should do the same. I'm not a fan of the style assuming what sizeof(bool) is.
> Source/JavaScriptCore/runtime/GenericArguments.h:66
> + static bool canDeletePropertyById(Type*, ExecState*, PropertyName);
> + static bool canDeletePropertyByIndex(Type*, ExecState*, unsigned);
These aren't implemented anywhere. Please delete.
> Source/JavaScriptCore/runtime/GenericArgumentsInlines.h:212
> + thisObject->setConfigurable(vm, index, descriptor.configurable());
We're guaranteed that it's already configurable before we call this? It seems like maybe we should bail out somewhere if the property is already non-configurable.
If i had to guess, maybe this program exhibits the bug:
function foo(x) {
Object.defineProperty(arguments, 0, {configurable:false, writable:true, value:20});
Object.defineProperty(arguments, 0, {configurable:true, writable:true, value:50}); // Does this succeed?
}
foo("foo")
> Source/JavaScriptCore/runtime/GenericArgumentsInlines.h:214
> + // Just overrride arguments (i.e finish aliasing) if its descriptor contains {writable: false}.
What do you mean by "finish aliasing" here?
> JSTests/microbenchmarks/super-getter.js:1
> +class B {
Please remove this file.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161009/c9c138e5/attachment.html>
More information about the webkit-unassigned
mailing list