We may add a ReadOnly property without setting the corresponding bit on Structure

[Saam Barati]

...

[Saam Barati]

<rdar://problem/27084591>

[Saam Barati]

Created attachment 283228 [details]
patch

[Saam Barati]

Comment on attachment 283228 [details]
patch

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

> Source/JavaScriptCore/runtime/JSObject.h:1569
> +    if (attributes & ReadOnly)
> +        structure->setContainsReadOnlyProperties();

I put this here to be consistent with JSObject::putDirectInternal,
but I almost feel like putting it inside Structure::addPropertyWithoutTransition
is a more natural place for it.

[Benjamin Poulain]

Comment on attachment 283228 [details]
patch

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

>> Source/JavaScriptCore/runtime/JSObject.h:1569
>> +        structure->setContainsReadOnlyProperties();
> 
> I put this here to be consistent with JSObject::putDirectInternal,
> but I almost feel like putting it inside Structure::addPropertyWithoutTransition
> is a more natural place for it.

I think that makes sense in addPropertyWithoutTransition() for both call sites. What's wrong with that?

[Saam Barati]

Comment on attachment 283228 [details]
patch

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

>>> Source/JavaScriptCore/runtime/JSObject.h:1569
>>> +        structure->setContainsReadOnlyProperties();
>> 
>> I put this here to be consistent with JSObject::putDirectInternal,
>> but I almost feel like putting it inside Structure::addPropertyWithoutTransition
>> is a more natural place for it.
> 
> I think that makes sense in addPropertyWithoutTransition() for both call sites. What's wrong with that?

I agree. I'm going to open another bug to consider this. I think we should also consider the Structure::addNewPropertyTransition to have it mark the bit on the new transition it's returning.

[Saam Barati]

I opened:
https://bugs.webkit.org/show_bug.cgi?id=159593

[WebKit Commit Bot]

Comment on attachment 283228 [details]
patch

Clearing flags on attachment: 283228

Committed r203015: <http://trac.webkit.org/changeset/203015>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.