[Webkit-unassigned] [Bug 159398] [test262] Fixing mapped arguments object property test case

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=159398

--- Comment #28 from Caio Lima <ticaiolima at gmail.com> ---
(In reply to comment #27)
> Comment on attachment 283766 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=283766&action=review
> 
> 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.

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.

> 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?

You are right. Fixing this case.

> Can you simplify the logic and duplicated code in
> GenericArguments<Type>::defineOwnProperty?

Yes. I agree that it can be better implemented.

> 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.
>
> > 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.

-- 
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/20160801/e064c543/attachment.html>