Review request for JDK-8147558: Add support for ES6 collections

Hannes Wallnoefer hannes.wallnoefer at oracle.com
Fri Feb 5 14:13:17 UTC 2016


Thanks for the review, Attila!

Am 2016-02-05 um 14:17 schrieb Attila Szegedi:
> Global.java:
> - Documentation for Global.setWeakSet() is wrong.
> - It’s kinda too bad we can’t generalize those getters with lazy sentinel and builtin getters. I guess we could if they weren’t stored in fields. Sounds like a good future use case for a VarHandle :-)
> - Global.initPrototype uses the “jdk.nashorn.internal.objects." string literal. It’s already used by Global.initConstructor too, maybe extract it into a constant? (FWIW, it also appears without the trailing dot in NashornLoader.java; maybe unify it into a single place somewhere?)

Thanks, will fix the documentation and clean up the string literals.

> NativeArray.java:
> - I’d refactor the common idiom in entries, keys, values, and getIterator into a separate method parametrized on iteration kind. FWIW, getIterator and values look exactly the same.

I'm not sure I understand. These are separate methods because each one 
defines a @Function for nasgen. Of course we could extend the @Function 
annotation to allow one Java method to define multiple JS functions. But 
these methods are one-liners anyway, so I doubt it would get much simpler.

> LinkedMap.java:
> - why does it need to be safe for concurrent use? Seems unnecessary to me.

You're right, it's not a requirement, but I think it's nice to have 
given the ubiquity of multithreading on the Java platform.

>
> WeakMap and WeakSet: didn’t you need the guarantees of LinkedMap with them too? I guess not.

The weak collections do not allow elements to be iterated over, so this 
isn't an issue there.

Hannes


More information about the nashorn-dev mailing list