1146979 - Cloning of variables in self-hosted code loses property attributes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

See bug 822080 comment 4.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: When cloning objects in self-hosting preserve the property attributes of the properties (or at least the readonly/configurable attributes)

Comment 2

[Till Schneidereit [:till]]

Comment on attachment 8582492 [details] [diff] [review]
When cloning objects in self-hosting preserve the property attributes of the properties (or at least the readonly/configurable attributes)

Review of attachment 8582492 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for fixing this. r=me with the comments addressed.

::: js/src/vm/SelfHosting.cpp
@@ +1245,5 @@
>  
> +    // Since all atoms used by self hosting are marked as permanent, any attempt
> +    // to look up a non-permanent atom will fail. We should only see such atoms
> +    // when code is looking for properties on the self hosted global which
> +    // aren't present.

SpiderMonkey uses 80 cols for comments, so please revert the reformatting here.

@@ +1294,5 @@
> +        if (!ids.append(shapes[i]->propid()))
> +            return false;
> +        // XXXbz if this is an accessor, we're going to turn it into a
> +        // value; is that ok?  If we can't assert that it's not an accessor,
> +        // then we can't just use shape->attributes().

This TODO should go. And I think shape->isAccessorShape() should be usable to assert against cloning accessors. (Which we might want to eventually do, but not right now.)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Done and https://hg.mozilla.org/integration/mozilla-inbound/rev/6796d08c38c1

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6796d08c38c1