8020718: RETURN symbol has wrong type in split functions

Reviewed-by: lagergren, attila

nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java

@@ -1754,7 +1754,7 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
             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 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
             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;
     }
 

nashorn/src/jdk/nashorn/internal/codegen/FinalizeTypes.java

@@ -649,8 +649,8 @@ final class FinalizeTypes extends NodeOperatorVisitor<LexicalContext> {
     }
 
     /**
-     * A symbol (and {@link Property}) can be tagged as "may be primitive". This is
+     * A symbol (and {@link jdk.nashorn.internal.runtime.Property}) can be tagged as "may be primitive".
-     * used a hint for dual fields that it is even worth it to try representing this
+     * 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 @@ final class FinalizeTypes extends NodeOperatorVisitor<LexicalContext> {
      * 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

nashorn/src/jdk/nashorn/internal/codegen/MethodEmitter.java

@@ -1281,10 +1281,14 @@ public class MethodEmitter implements Emitter {
     }
 
     MethodEmitter registerReturn() {
-        this.hasReturn = true;
+        setHasReturn();
         return this;
     }
 
+    void setHasReturn() {
+        this.hasReturn = true;
+    }
+
     /**
      * Perform a non void return, popping the type from the stack
      *

nashorn/src/jdk/nashorn/internal/codegen/SplitMethodEmitter.java

@@ -77,15 +77,15 @@ public class SplitMethodEmitter extends MethodEmitter {
         }
 
         if (lc.isExternalTarget(splitNode, label)) {
-             externalTargets.add(label);
+            externalTargets.add(label);
-             return externalTargets.size() - 1;
+            return externalTargets.size() - 1;
-         }
+        }
-         return -1;
+        return -1;
     }
 
     @Override
     MethodEmitter registerReturn() {
-        super.registerReturn();
+        setHasReturn();
         loadCompilerConstant(SCOPE);
         checkcast(Scope.class);
         load(0);

nashorn/src/jdk/nashorn/internal/ir/Block.java

@@ -33,10 +33,14 @@ import java.util.Comparator;
 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 @@ public class Block extends Node implements BreakableNode, Flags<Block> {
         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);

nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java

@@ -817,6 +817,7 @@ public final class FunctionNode extends LexicalContextExpression implements Flag
         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 @@ public final class FunctionNode extends LexicalContextExpression implements Flag
                 lastToken,
                 flags,
                 name,
-                Type.widest(this.returnType, returnType.isObject() ?
+                type,
-                    Type.OBJECT :
-                    returnType),
                 compileUnit,
                 compilationState,
-                body,
+                body.setReturnType(type),
                 parameters,
                 snapshot,
                 hints));

nashorn/src/jdk/nashorn/internal/ir/IdentNode.java

@@ -153,14 +153,27 @@ public final class IdentNode extends Expression implements PropertyKey, TypeOver
     }
 
     /**
-     * We can only override type if the symbol lives in the scope, otherwise
+     * We can only override type if the symbol lives in the scope, as otherwise
-     * it is strongly determined by the local variable already allocated
+     * 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());
     }
 
     /**

nashorn/src/jdk/nashorn/internal/ir/Symbol.java

@@ -35,6 +35,8 @@ import jdk.nashorn.internal.runtime.Context;
 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.
  */
@@ -441,6 +443,14 @@ public final class Symbol implements Comparable<Symbol> {
         this.range = range;
     }
 
+    /**
+     * 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