nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java
changeset 21457 381acbd07fe5
parent 21449 72d51df5ed85
child 21462 44238d753963
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Wed Oct 23 20:15:43 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Fri Oct 25 10:20:49 2013 +0200
@@ -359,8 +359,11 @@
         return load(node, node.hasType() ? node.getType() : null, false);
     }
 
-    private static boolean safeLiteral(final Expression rhs) {
-        return rhs instanceof LiteralNode && !(rhs instanceof ArrayLiteralNode);
+    // Test whether conversion from source to target involves a call of ES 9.1 ToPrimitive
+    // with possible side effects from calling an object's toString or valueOf methods.
+    private boolean noToPrimitiveConversion(final Type source, final Type target) {
+        // Object to boolean conversion does not cause ToPrimitive call
+        return source.isJSPrimitive() || !target.isJSPrimitive() || target.isBoolean();
     }
 
     MethodEmitter loadBinaryOperands(final Expression lhs, final Expression rhs, final Type type) {
@@ -374,25 +377,19 @@
         // can combine a LOAD with a CONVERT operation (e.g. use a dynamic getter with the conversion target type as its
         // return value). What we do here is reorder LOAD RIGHT and CONVERT LEFT when possible; it is possible only when
         // we can prove that executing CONVERT LEFT can't have a side effect that changes the value of LOAD RIGHT.
-        // Basically, if we know that either LEFT is not an object, or RIGHT is a constant literal, then we can do the
+        // Basically, if we know that either LEFT already is a primitive value, or does not have to be converted to
+        // a primitive value, or RIGHT is an expression that loads without side effects, then we can do the
         // reordering and collapse LOAD/CONVERT into a single operation; otherwise we need to do the more costly
         // separate operations to preserve specification semantics.
-        final Type lhsType = lhs.getType();
-        if (lhsType.isObject() && !safeLiteral(rhs)) {
-            // Can't reorder. Load and convert separately.
-            load(lhs, lhsType, baseAlreadyOnStack);
-            load(rhs, rhs.getType(), false);
-            // Avoid empty SWAP, SWAP bytecode sequence if CONVERT LEFT is a no-op
-            if (!lhsType.isEquivalentTo(type)) {
-                method.swap();
-                method.convert(type);
-                method.swap();
-            }
-            method.convert(type);
-        } else {
+        if (noToPrimitiveConversion(lhs.getType(), type) || rhs.isLocal()) {
             // Can reorder. Combine load and convert into single operations.
             load(lhs, type, baseAlreadyOnStack);
             load(rhs, type, false);
+        } else {
+            // Can't reorder. Load and convert separately.
+            load(lhs, lhs.getType(), baseAlreadyOnStack);
+            load(rhs, rhs.getType(), false);
+            method.swap().convert(type).swap().convert(type);
         }
 
         return method;