8038396: fix for the compiler expression evaluator to be more inquisitive about types
authorattila
Wed, 26 Mar 2014 15:00:32 +0100
changeset 24730 505fb524e688
parent 24729 2b13051f2122
child 24731 ab0c8fc915ae
8038396: fix for the compiler expression evaluator to be more inquisitive about types Reviewed-by: hannesw, lagergren
nashorn/src/jdk/nashorn/internal/codegen/Attr.java
nashorn/src/jdk/nashorn/internal/codegen/CompilationEnvironment.java
--- a/nashorn/src/jdk/nashorn/internal/codegen/Attr.java	Mon Mar 24 18:41:06 2014 +0100
+++ b/nashorn/src/jdk/nashorn/internal/codegen/Attr.java	Wed Mar 26 15:00:32 2014 +0100
@@ -58,7 +58,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-
 import jdk.nashorn.internal.codegen.types.Type;
 import jdk.nashorn.internal.ir.AccessNode;
 import jdk.nashorn.internal.ir.BinaryNode;
@@ -354,7 +353,11 @@
     }
 
     private boolean useOptimisticTypes() {
-        return env.useOptimisticTypes() && !lc.isInSplitNode();
+        return env.useOptimisticTypes() &&
+                // an inner function in on-demand compilation is not compiled, so no need to evaluate expressions in it
+                (!env.isOnDemandCompilation() || lc.getOutermostFunction() == lc.getCurrentFunction()) &&
+                // No optimistic compilation within split nodes for now
+                !lc.isInSplitNode();
     }
 
     @Override
@@ -1169,7 +1172,6 @@
                 tagNeverOptimistic(binaryNode.rhs());
             }
         }
-        //tagOptimistic(binaryNode.lhs());
         tagOptimistic(binaryNode.rhs());
 
         return true;
@@ -1221,6 +1223,8 @@
 
     @Override
     public boolean enterASSIGN(final BinaryNode binaryNode) {
+        // left hand side of an ordinary assignment need never be optimistic (it's written only, not read).
+        tagNeverOptimistic(binaryNode.lhs());
         return enterAssignmentNode(binaryNode);
     }
 
@@ -2068,7 +2072,10 @@
         // if we are running with optimistic types, this starts out as e.g. int, and based on previous
         // failed assumptions it can be wider, for example double if we have failed this assumption
         // in a previous run
-        Type optimisticType = getOptimisticType(node);
+        final boolean isNeverOptimistic = isTaggedNeverOptimistic(node);
+
+        // avoid optimistic type evaluation if the node is never optimistic
+        Type optimisticType = isNeverOptimistic ? node.getMostPessimisticType() : getOptimisticType(node);
 
         if (argumentsType != null) {
             optimisticType = Type.widest(optimisticType, argumentsType);
@@ -2082,7 +2089,7 @@
             return expr;
         }
 
-        if (isTaggedNeverOptimistic(expr)) {
+        if (isNeverOptimistic) {
             return expr;
         }
 
--- a/nashorn/src/jdk/nashorn/internal/codegen/CompilationEnvironment.java	Mon Mar 24 18:41:06 2014 +0100
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CompilationEnvironment.java	Wed Mar 26 15:00:32 2014 +0100
@@ -287,19 +287,6 @@
     }
 
     /**
-     * Check if a program point was invalidated during a previous run
-     * of this method, i.e. we did an optimistic assumption that now is wrong.
-     * This is the basis of generating a wider type. getOptimisticType
-     * in this class will query for invalidation and suggest a wider type
-     * upon recompilation if this info exists.
-     * @param programPoint program point to check
-     * @return true if it was invalidated during a previous run
-     */
-    boolean isInvalidated(final int programPoint) {
-        return invalidatedProgramPoints.get(programPoint) != null;
-    }
-
-    /**
      * Get the parameter type at a parameter position if known from previous runtime calls
      * or optimistic profiles.
      *
@@ -393,7 +380,7 @@
             }
             return evaluatedType;
         }
-        return node.getMostOptimisticType();
+        return mostOptimisticType;
     }
 
 
@@ -429,8 +416,25 @@
         if(find == null) {
             return null;
         }
-        final Class<?> clazz = find.getProperty().getCurrentType();
-        return clazz == null ? null : Type.typeFor(clazz);
+
+        final Property property = find.getProperty();
+        final Class<?> propertyClass = property.getCurrentType();
+        if (propertyClass == null) {
+            // propertyClass == null means its Undefined, which is object
+            return Type.OBJECT;
+        } else if (propertyClass.isPrimitive()) {
+            return Type.typeFor(propertyClass);
+        }
+
+        final ScriptObject owner = find.getOwner();
+        if(property.hasGetterFunction(owner)) {
+            // Can have side effects, so we can't safely evaluate it; since !propertyClass.isPrimitive(), it's Object.
+            return Type.OBJECT;
+        }
+
+        // Safely evaluate the property, and return the narrowest type for the actual value (e.g. Type.INT for a boxed
+        // integer).
+        return Type.typeFor(ObjectClassGenerator.unboxedFieldType(property.getObjectValue(owner, owner)));
     }
 
     private Object evaluateSafely(Expression expr) {