Review request for JDK-8072426
attila.szegedi at oracle.com
Sun Mar 8 13:29:32 UTC 2015
Thanks for the reviews. In order to backport this to 8u-dev, I had to make some changes. Namely, since we introduced a default method into the JSObject interface, we could not compile this under jdk8u-dev, as for JDK 8 Nashorn is compiled with "-source 7" as the part of the build process is running Nasgen, which is done with the boot JDK, which is a JDK 7 for 8u[*].
For this reason, the 8u backport doesn't add a new getDefaultValue() method to the JSObject interface, as that – without the default methods' ability – wouldn't be a binary compatible class change. Instead, AbstractJSObject gains a getDefaultValue(hint) method with the same behavior as well as a static helper getDefaultValue(jsobj, hint) that delegates to AbstractJSObject.getDefaultValue() for AbstractJSObject, and runs the generic algorithm for any other JSObject. (As a separate task, we'll need to forward-port this static helper into JDK 9 codebase so it stays compatible with 8; I'll soon file a JIRA for that.)
With these in mind, please review the backport changeset at <http://cr.openjdk.java.net/~attila/8072426/webrev.8u>
[*] This raises an interesting further issue, namely that this somewhat peculiar dependency disallows use of any Java 8 syntactic construct in Nashorn code destined for 8. Long term, it'd be great if we could either get 8u to use the previous 8 release as its boot JDK, or if we could rewrite Nasgen so that it reads Nashorn .class files, but doesn't load them into the JVM.
On Feb 20, 2015, at 9:48 AM, Hannes Wallnoefer <hannes.wallnoefer at oracle.com> wrote:
> Nice work!
> Am 2015-02-19 um 18:48 schrieb Attila Szegedi:
>> Please review JDK-8072426 at <http://cr.openjdk.java.net/~attila/8072426/webrev.00> for <https://bugs.openjdk.java.net/browse/JDK-8072426>
>> This is what basically became an epic yak shaving exercise after we determined that POJOs aren't subjected to ToPrimitive (earlier known as java.math.RoundingMode.UP != "UP" problem). While we originally advocated for throwing TypeError for POJOs [[DefaultValue]] that would've prevented their implicit toString() in string concatenation too. Instead, I opted to fix this once and forever but it turned out that the biggest issue with it is the fact that JSObject was also not handled correctly for ToPrimitive; fixing that turned out to be the most challenging part, introducing a new JSObject.getDefaultValue() method and deprecating JSObject.toNumber() (which isn't used anymore at all).
>> As an unexpected boon, JSObjectLinker no longer needs to do any conversions, so its GuardingTypeConverterFactory functionality has been entirely removed. NashornPrimitiveLinker will now automatically take care of converting JSObject instances to various primitive types, as per specification behavior.
>> Finally, the ScriptRuntime changes to the equals() and equalDifferentType() are purely refactoring of some common type testing subexpressions and conversions, as well as short-circuiting of some comparisons. They aren't strictly necessary for this issue, but just for clear understanding of what's happening it seemed like a good idea to make them more concise (ScriptRuntime.EQ and .NE, of which the two affected methods are sub-parts are also not invoked terribly often, and I'll soon make them be invoked even less frequently.)
More information about the nashorn-dev