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