[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 #81 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 294029
  --> https://bugs.webkit.org/attachment.cgi?id=294029
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=294029&action=review

patch LGTM. Just a couple comments. Also, I think the names "modified" and "overrides" are somewhat confusing. I want us to try to pick a better name to indicate that they mean.
- Overrides => argument is mapped/unmapped
- Modified => the descriptor is now changed, however, the argument may or may not be mapped

> Source/JavaScriptCore/runtime/DirectArguments.cpp:106
> +    if (thisObject->m_modifiedArguments)
> +        visitor.markAuxiliary(thisObject->m_modifiedArguments.get());

This should be done in the GenericArguments visitChildren method since GenericArguments owns that field. You may need to write a visitCHildren for GenericArguments.

> Source/JavaScriptCore/runtime/GenericArgumentsInlines.h:226
> +        if (optionalIndex && thisObject->canAccessIndexQuickly(optionalIndex.value())) {

Nit: You can move this branch into the above `if (optionalIndex)` statement and drop the "optionalIndex" part.

> Source/JavaScriptCore/runtime/GenericArgumentsInlines.h:239
> +                thisObject->overrideArgument(vm, index);

We don't need to set modified here too?

> Source/JavaScriptCore/runtime/PropertyDescriptor.h:82
> +

please revert

> Source/JavaScriptCore/runtime/ScopedArguments.cpp:116
> +    if (thisObject->m_modifiedArguments)
> +        visitor.markAuxiliary(thisObject->m_modifiedArguments.get());

Ditto for this visitChildren

-- 
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/20161121/0b89cfc4/attachment.html>