<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [test262] Fixing mapped arguments object property test case"
href="https://bugs.webkit.org/show_bug.cgi?id=159398#c27">Comment # 27</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [test262] Fixing mapped arguments object property test case"
href="https://bugs.webkit.org/show_bug.cgi?id=159398">bug 159398</a>
from <span class="vcard"><a class="email" href="mailto:ggaren@apple.com" title="Geoffrey Garen <ggaren@apple.com>"> <span class="fn">Geoffrey Garen</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=283766&action=diff" name="attach_283766" title="Patch">attachment 283766</a> <a href="attachment.cgi?id=283766&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=283766&action=review">https://bugs.webkit.org/attachment.cgi?id=283766&action=review</a>
I think I understand the logic here now: It used to be that a property either aliased *or* lived in JSObject property storage with attributes. You're adding a new feature that allows a property to alias *and* live in JSObject property storage with attributes. In this state, somewhat confusingly, the value stored in JSObject property storage can be bogus because it is not observed. You should explain this in your ChangeLog.
I think you missed a case where the new property descriptor is not an accessor, is writable, and is DontDelete. In that case, your code will return early without setting the DontDelete attribute. Is that right?
Can you simplify the logic and duplicated code in GenericArguments<Type>::defineOwnProperty?
The relevant cases inside canAccessIndexQuickly, as I understand them, are:
(1) If the property is not an accessor and has either no attributes or only the writable:true attribute, set the value and return.
Otherwise:
(2) getIndexQuickly and putDirectMayBeIndex.
(3) If the property has any attributes other than configurable:true or configurable:false, overrideArgument.
(4) Fall through to the standard call to defineOwnProperty.
<span class="quote">> Source/JavaScriptCore/runtime/GenericArgumentsInlines.h:158
> + PropertySlot slot(thisObject, PropertySlot::InternalMethodType::GetOwnProperty);
> + if (Base::getOwnPropertySlot(thisObject, exec, ident, slot)) {
> + if (slot.attributes() & DontDelete)
> + return false;
> + }</span >
Let's make this a helper function: GenericArguments<T>::canDeleteProperty(PropertyName).
<span class="quote">> Source/JavaScriptCore/runtime/GenericArgumentsInlines.h:179
> + PropertySlot slot(thisObject, PropertySlot::InternalMethodType::GetOwnProperty);
> + if (Base::getOwnPropertySlotByIndex(thisObject, exec, index, slot)) {
> + if (slot.attributes() & DontDelete)
> + return false;
> + }</span >
Ditto.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>