RFR: 8156743: ES6 for..of should work for Java Maps and Sets
srinivas.dama at oracle.com
Thu Mar 2 16:39:48 UTC 2017
Thank you for the detailed review.
From: Attila Szegedi [mailto:szegedia at gmail.com]
Sent: Thursday, March 02, 2017 9:01 PM
To: Srinivas Dama
Subject: Re: RFR: 8156743: ES6 for..of should work for Java Maps and Sets
Contrary to the issue name, I don’t see anything for handling Set in the patch, but I guess that’s because Set is a Collection and those already work?
As an aside… I was looking into your use of NativeJava.from to convert a Java array to a NativeArray, and realized that we have at least 3 ways to do it, each with different drawbacks:
1. NativeJava.from as used in this patch. Its drawback is that it defensively always clones the array (here we woulnd’t need to clone it, although it doesn’t hurt) 2. Global.instance.wrapAsObject. Its drawback is that its Object case is further down the if/else list than NativeJava.from (5th instanceof vs. 2nd) 3. Global.instance.allocate. (This one is primarily used from generated bytecode.) Its drawback is that it iterates over the elements scanning for any ScriptRuntime.EMPTY values. I guess we could pass in a boolean flag to indicate there are no empty values to skip this scan (compiler could set the flag when it can prove the array literal it’s constructing has no empties).
This doesn’t affect your patch - I think NativeJava.from() is a decent choice.
> On 02 Mar 2017, at 15:36, Srinivas Dama <srinivas.dama at oracle.com> wrote:
> Please review http://cr.openjdk.java.net/~sdama/8156743/webrev.00/
> for https://bugs.openjdk.java.net/browse/JDK-8156743
More information about the nashorn-dev