RFR: 8159589: ScriptObjectMirror.clear() should remove all enumerable properties

[Hannes Wallnöfer]

Thanks for the review, Sundar. Replies inline below.

> Am 20.06.2016 um 08:35 schrieb Sundararajan Athijegannathan <sundararajan.athijegannathan at oracle.com>:
> 
> Few questions:
> 
> 1) So, we are allowing delete on non-configurable properties. Is this
> fine from optimization PoV? i.e., we can't assume that a
> non-configurable property can not be deleted.

This is indeed a problem. I wrote a test where clear is invoked on the global object between evaluation of the same script, and callsites are not invalidated properly. I don’t think it is worth to change the way global variable callsites are guarded for this change, so I’m withdrawing this RFR and closing the issue. 

Thanks for spotting this, Sundar.  

> 2) ScriptObject.clear - it appears previously, it deleted inherited
> properties too. Now only immediate properties. Intended? do we have
> tests to cover these cases?
> 

No, we previously/currently also only delete properties on the object itself. Even though we use a property iterator that includes inherited properties, the deletion will only take place for properties on the object itself. I originally thought this was just an inefficiency, but now that I think of it actually is a bug as it could result in removal of non-enumerable properties. I’ll file an Jira issue for it.

> 3) Global's clear method clears lexicalScope. Do we need a test for ES6
> lexical scope clear?

True, but not a problem anymore since I’m not following this further.

Hannes


> -Sundar
> 
> On 6/17/2016 7:24 PM, Hannes Wallnöfer wrote:
>> Please review JDK-8159589: ScriptObjectMirror.clear() should remove all enumerable properties:
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8159589
>> Webrev: http://cr.openjdk.java.net/~hannesw/8159589/webrev.00/
>> 
>> In our current implementation, ScriptObjectMirror.clear() observes JS semantics by not clearing properties that are non-configurable, and even throwing type error when invoking clear() on an object that contains such properties.
>> 
>> IMO, there’s no need to follow JS semantics since we’re calling the method from outside of JS. With this change, clear() will remove all enumerable properties. This also makes the method consistent with size() and isEmpty(), which previously could return values > 0 and false after calling clear().
>> 
>> On a practical note, this is really convenient with global objects as it can be used to return globals to their initial state after an eval, removing all declared vars and functions but leaving the (non-enumerable) built-ins.
>> 
>> Hannes
>