<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#c28">Comment # 28</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:ticaiolima@gmail.com" title="Caio Lima <ticaiolima@gmail.com>"> <span class="fn">Caio Lima</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=159398#c27">comment #27</a>)
<span class="quote">> 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.</span >
Yes. My change is using the attributes just to check the attribute is DontDelete in deleteProperty*. One alternative is store configurable attribute in an auxiliary structure (such as m_overrides), however, IMHO, it doesn't look as intuitive as checking property attributes.
<span class="quote">> 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?</span >
You are right. Fixing this case.
<span class="quote">> Can you simplify the logic and duplicated code in
> GenericArguments<Type>::defineOwnProperty?</span >
Yes. I agree that it can be better implemented.
<span class="quote">> 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 >
>
<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;
> > + }
>
> Let's make this a helper function:
> GenericArguments<T>::canDeleteProperty(PropertyName).
>
> > 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;
> > + }
>
> Ditto.</span ></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>