8020718: RETURN symbol has wrong type in split functions
authorhannesw
Wed, 24 Jul 2013 13:16:34 +0200 (2013-07-24)
changeset 19095 0b215bda525d
parent 19094 40665ad691ca
child 19096 21add09922c5
8020718: RETURN symbol has wrong type in split functions Reviewed-by: lagergren, attila
nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java
nashorn/src/jdk/nashorn/internal/codegen/FinalizeTypes.java
nashorn/src/jdk/nashorn/internal/codegen/MethodEmitter.java
nashorn/src/jdk/nashorn/internal/codegen/SplitMethodEmitter.java
nashorn/src/jdk/nashorn/internal/ir/Block.java
nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java
nashorn/src/jdk/nashorn/internal/ir/IdentNode.java
nashorn/src/jdk/nashorn/internal/ir/Symbol.java
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Wed Jul 24 12:48:09 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Wed Jul 24 13:16:34 2013 +0200
@@ -1754,7 +1754,7 @@
             caller.ifne(breakLabel);
             //has to be zero
             caller.label(new Label("split_return"));
-            method.loadCompilerConstant(RETURN);
+            caller.loadCompilerConstant(RETURN);
             caller._return(lc.getCurrentFunction().getReturnType());
             caller.label(breakLabel);
         } else {
@@ -1785,6 +1785,11 @@
             caller.label(breakLabel);
         }
 
+        // If split has a return and caller is itself a split method it needs to propagate the return.
+        if (hasReturn) {
+            caller.setHasReturn();
+        }
+
         return splitNode;
     }
 
--- a/nashorn/src/jdk/nashorn/internal/codegen/FinalizeTypes.java	Wed Jul 24 12:48:09 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/FinalizeTypes.java	Wed Jul 24 13:16:34 2013 +0200
@@ -649,8 +649,8 @@
     }
 
     /**
-     * A symbol (and {@link Property}) can be tagged as "may be primitive". This is
-     * used a hint for dual fields that it is even worth it to try representing this
+     * A symbol (and {@link jdk.nashorn.internal.runtime.Property}) can be tagged as "may be primitive".
+     * This is used a hint for dual fields that it is even worth it to try representing this
      * field as something other than java.lang.Object.
      *
      * @param node node in which to tag symbols as primitive
@@ -856,7 +856,7 @@
      * if the expression type changes.
      *
      * Assignments use their lhs as node symbol, and in this case we can't modify
-     * it. Then {@link CodeGenerator#Store} needs to do an explicit conversion.
+     * it. Then {@link CodeGenerator.Store} needs to do an explicit conversion.
      * This is happens very rarely.
      *
      * @param node
--- a/nashorn/src/jdk/nashorn/internal/codegen/MethodEmitter.java	Wed Jul 24 12:48:09 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/MethodEmitter.java	Wed Jul 24 13:16:34 2013 +0200
@@ -1281,8 +1281,12 @@
     }
 
     MethodEmitter registerReturn() {
+        setHasReturn();
+        return this;
+    }
+
+    void setHasReturn() {
         this.hasReturn = true;
-        return this;
     }
 
     /**
--- a/nashorn/src/jdk/nashorn/internal/codegen/SplitMethodEmitter.java	Wed Jul 24 12:48:09 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/SplitMethodEmitter.java	Wed Jul 24 13:16:34 2013 +0200
@@ -77,15 +77,15 @@
         }
 
         if (lc.isExternalTarget(splitNode, label)) {
-             externalTargets.add(label);
-             return externalTargets.size() - 1;
-         }
-         return -1;
+            externalTargets.add(label);
+            return externalTargets.size() - 1;
+        }
+        return -1;
     }
 
     @Override
     MethodEmitter registerReturn() {
-        super.registerReturn();
+        setHasReturn();
         loadCompilerConstant(SCOPE);
         checkcast(Scope.class);
         load(0);
--- a/nashorn/src/jdk/nashorn/internal/ir/Block.java	Wed Jul 24 12:48:09 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/ir/Block.java	Wed Jul 24 13:16:34 2013 +0200
@@ -33,10 +33,14 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+
 import jdk.nashorn.internal.codegen.Label;
+import jdk.nashorn.internal.codegen.types.Type;
 import jdk.nashorn.internal.ir.annotations.Immutable;
 import jdk.nashorn.internal.ir.visitor.NodeVisitor;
 
+import static jdk.nashorn.internal.codegen.CompilerConstants.RETURN;
+
 /**
  * IR representation for a list of statements.
  */
@@ -212,6 +216,19 @@
         return isTerminal ? setFlag(lc, IS_TERMINAL) : clearFlag(lc, IS_TERMINAL);
     }
 
+    /**
+     * Set the type of the return symbol in this block if present.
+     * @param returnType the new type
+     * @return this block
+     */
+    public Block setReturnType(final Type returnType) {
+        final Symbol symbol = getExistingSymbol(RETURN.symbolName());
+        if (symbol != null) {
+            symbol.setTypeOverride(returnType);
+        }
+        return this;
+    }
+
     @Override
     public boolean isTerminal() {
         return getFlag(IS_TERMINAL);
--- a/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java	Wed Jul 24 12:48:09 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java	Wed Jul 24 13:16:34 2013 +0200
@@ -817,6 +817,7 @@
         if (this.returnType == returnType) {
             return this;
         }
+        final Type type = Type.widest(this.returnType, returnType.isObject() ? Type.OBJECT : returnType);
         return Node.replaceInLexicalContext(
             lc,
             this,
@@ -825,12 +826,10 @@
                 lastToken,
                 flags,
                 name,
-                Type.widest(this.returnType, returnType.isObject() ?
-                    Type.OBJECT :
-                    returnType),
+                type,
                 compileUnit,
                 compilationState,
-                body,
+                body.setReturnType(type),
                 parameters,
                 snapshot,
                 hints));
--- a/nashorn/src/jdk/nashorn/internal/ir/IdentNode.java	Wed Jul 24 12:48:09 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/ir/IdentNode.java	Wed Jul 24 13:16:34 2013 +0200
@@ -153,14 +153,27 @@
     }
 
     /**
-     * We can only override type if the symbol lives in the scope, otherwise
-     * it is strongly determined by the local variable already allocated
+     * We can only override type if the symbol lives in the scope, as otherwise
+     * it is strongly determined by the local variable already allocated.
+     *
+     * <p>We also return true if the symbol represents the return value of a function with a
+     * non-generic return type as in this case we need to propagate the type instead of
+     * converting to object, for example if the symbol is used as the left hand side of an
+     * assignment such as in the code below.</p>
+     *
+     * <pre>{@code
+     *   try {
+     *     return 2;
+     *   } finally {
+     *     return 3;
+     *   }
+     * }</pre>
      *
      * @return true if can have callsite type
      */
     @Override
     public boolean canHaveCallSiteType() {
-        return getSymbol() != null && getSymbol().isScope();
+        return getSymbol() != null && (getSymbol().isScope() || getSymbol().isNonGenericReturn());
     }
 
     /**
--- a/nashorn/src/jdk/nashorn/internal/ir/Symbol.java	Wed Jul 24 12:48:09 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/ir/Symbol.java	Wed Jul 24 13:16:34 2013 +0200
@@ -35,6 +35,8 @@
 import jdk.nashorn.internal.runtime.Debug;
 import jdk.nashorn.internal.runtime.options.Options;
 
+import static jdk.nashorn.internal.codegen.CompilerConstants.RETURN;
+
 /**
  * Maps a name to specific data.
  */
@@ -442,6 +444,14 @@
     }
 
     /**
+     * Check if this symbol represents a return value with a known non-generic type.
+     * @return true if specialized return value
+     */
+    public boolean isNonGenericReturn() {
+        return getName().equals(RETURN.symbolName()) && type != Type.OBJECT;
+    }
+
+    /**
      * Check if this symbol is a function parameter of known
      * narrowest type
      * @return true if parameter