8034954: Optimistic iteration in for-in and for-each

Reviewed-by: hannesw, lagergren

4 files changed, 58 insertions, 10 deletions

diff --git a/src/jdk/nashorn/internal/codegen/CodeGenerator.java b/src/jdk/nashorn/internal/codegen/CodeGenerator.java
index cfdb64bd..97b26212 100644
--- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java
+++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java
@@ -1622,9 +1622,18 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
 
             @Override
             protected void evaluate() {
-                method.load(ITERATOR_TYPE, iterSlot);
-                // TODO: optimistic for-in iteration
-                method.invoke(interfaceCallNoLookup(ITERATOR_CLASS, "next", Object.class));
+                new OptimisticOperation((Optimistic)forNode.getInit(), TypeBounds.UNBOUNDED) {
+                    @Override
+                    void loadStack() {
+                        method.load(ITERATOR_TYPE, iterSlot);
+                    }
+
+                    @Override
+                    void consumeStack() {
+                        method.invoke(interfaceCallNoLookup(ITERATOR_CLASS, "next", Object.class));
+                        convertOptimisticReturnValue();
+                    }
+                }.emit();
             }
         }.store();
         body.accept(this);
diff --git a/src/jdk/nashorn/internal/codegen/Compiler.java b/src/jdk/nashorn/internal/codegen/Compiler.java
index bb59eb5f..86a6c231 100644
--- a/src/jdk/nashorn/internal/codegen/Compiler.java
+++ b/src/jdk/nashorn/internal/codegen/Compiler.java
@@ -53,6 +53,7 @@ import java.util.logging.Level;
 import jdk.internal.dynalink.support.NameCodec;
 import jdk.nashorn.internal.codegen.ClassEmitter.Flag;
 import jdk.nashorn.internal.codegen.types.Type;
+import jdk.nashorn.internal.ir.Expression;
 import jdk.nashorn.internal.ir.FunctionNode;
 import jdk.nashorn.internal.ir.Optimistic;
 import jdk.nashorn.internal.ir.debug.ClassHistogramElement;
@@ -65,6 +66,7 @@ import jdk.nashorn.internal.runtime.ParserException;
 import jdk.nashorn.internal.runtime.RecompilableScriptFunctionData;
 import jdk.nashorn.internal.runtime.ScriptEnvironment;
 import jdk.nashorn.internal.runtime.ScriptObject;
+import jdk.nashorn.internal.runtime.ScriptRuntime;
 import jdk.nashorn.internal.runtime.Source;
 import jdk.nashorn.internal.runtime.logging.DebugLogger;
 import jdk.nashorn.internal.runtime.logging.Loggable;
@@ -473,6 +475,19 @@ public final class Compiler implements Loggable {
         return typeEvaluator.getOptimisticType(node);
     }
 
+    /**
+     * Returns true if the expression can be safely evaluated, and its value is an object known to always use
+     * String as the type of its property names retrieved through
+     * {@link ScriptRuntime#toPropertyIterator(Object)}. It is used to avoid optimistic assumptions about its
+     * property name types.
+     * @param expr the expression to test
+     * @return true if the expression can be safely evaluated, and its value is an object known to always use
+     * String as the type of its property iterators.
+     */
+    boolean hasStringPropertyIterator(final Expression expr) {
+        return typeEvaluator.hasStringPropertyIterator(expr);
+    }
+
     void addInvalidatedProgramPoint(final int programPoint, final Type type) {
         invalidatedProgramPoints.put(programPoint, type);
     }
diff --git a/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java b/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java
index 130bd66f..995b29a6 100644
--- a/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java
+++ b/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java
@@ -551,13 +551,19 @@ final class LocalVariableTypesCalculator extends NodeVisitor<LexicalContext>{
 
         final Expression init = forNode.getInit();
         if(forNode.isForIn()) {
-            forNode.getModify().accept(this);
-            enterTestFirstLoop(forNode, null, init);
+            final JoinPredecessorExpression iterable = forNode.getModify();
+            iterable.accept(this);
+            enterTestFirstLoop(forNode, null, init,
+                    // If we're iterating over property names, and we can discern from the runtime environment
+                    // of the compilation that the object being iterated over must use strings for property
+                    // names (e.g., it is a native JS object or array), then we'll not bother trying to treat
+                    // the property names optimistically.
+                    !forNode.isForEach() && compiler.hasStringPropertyIterator(iterable.getExpression()));
         } else {
             if(init != null) {
                 init.accept(this);
             }
-            enterTestFirstLoop(forNode, forNode.getModify(), null);
+            enterTestFirstLoop(forNode, forNode.getModify(), null, false);
         }
         return false;
     }
@@ -792,7 +798,8 @@ final class LocalVariableTypesCalculator extends NodeVisitor<LexicalContext>{
         return false;
     }
 
-    private void enterTestFirstLoop(final LoopNode loopNode, final JoinPredecessorExpression modify, final Expression iteratorValues) {
+    private void enterTestFirstLoop(final LoopNode loopNode, final JoinPredecessorExpression modify,
+            final Expression iteratorValues, final boolean iteratorValuesAreObject) {
         final JoinPredecessorExpression test = loopNode.getTest();
         if(isAlwaysFalse(test)) {
             test.accept(this);
@@ -814,8 +821,12 @@ final class LocalVariableTypesCalculator extends NodeVisitor<LexicalContext>{
                 jumpToLabel(test, breakLabel);
             }
             if(iteratorValues instanceof IdentNode) {
-                // Receives iterator values; they're currently all objects (JDK-8034954).
-                onAssignment((IdentNode)iteratorValues, LvarType.OBJECT);
+                final IdentNode ident = (IdentNode)iteratorValues;
+                // Receives iterator values; the optimistic type of the iterator values is tracked on the
+                // identifier, but we override optimism if it's known that the object being iterated over will
+                // never have primitive property names.
+                onAssignment(ident, iteratorValuesAreObject ? LvarType.OBJECT :
+                    toLvarType(compiler.getOptimisticType(ident)));
             }
             final Block body = loopNode.getBody();
             body.accept(this);
@@ -955,7 +966,7 @@ final class LocalVariableTypesCalculator extends NodeVisitor<LexicalContext>{
         if(whileNode.isDoWhile()) {
             enterDoWhileLoop(whileNode);
         } else {
-            enterTestFirstLoop(whileNode, null, null);
+            enterTestFirstLoop(whileNode, null, null, false);
         }
         return false;
     }
diff --git a/src/jdk/nashorn/internal/codegen/TypeEvaluator.java b/src/jdk/nashorn/internal/codegen/TypeEvaluator.java
index ea58d0a9..11a23ecd 100644
--- a/src/jdk/nashorn/internal/codegen/TypeEvaluator.java
+++ b/src/jdk/nashorn/internal/codegen/TypeEvaluator.java
@@ -55,6 +55,19 @@ final class TypeEvaluator {
         this.runtimeScope = runtimeScope;
     }
 
+    /**
+     * Returns true if the expression can be safely evaluated, and its value is an object known to always use
+     * String as the type of its property names retrieved through
+     * {@link ScriptRuntime#toPropertyIterator(Object)}. It is used to avoid optimistic assumptions about its
+     * property name types.
+     * @param expr the expression to test
+     * @return true if the expression can be safely evaluated, and its value is an object known to always use
+     * String as the type of its property iterators.
+     */
+    boolean hasStringPropertyIterator(final Expression expr) {
+        return evaluateSafely(expr) instanceof ScriptObject;
+    }
+
     Type getOptimisticType(final Optimistic node) {
         assert compiler.useOptimisticTypes();