# HG changeset patch # User hannesw # Date 1374664594 -7200 # Node ID 0b215bda525d1e723727b1b7fef42c98c600ffe7 # Parent 40665ad691ca11609f91a87f992656f61e982820 8020718: RETURN symbol has wrong type in split functions Reviewed-by: lagergren, attila diff -r 40665ad691ca -r 0b215bda525d nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.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; } diff -r 40665ad691ca -r 0b215bda525d nashorn/src/jdk/nashorn/internal/codegen/FinalizeTypes.java --- 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 diff -r 40665ad691ca -r 0b215bda525d nashorn/src/jdk/nashorn/internal/codegen/MethodEmitter.java --- 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; } /** diff -r 40665ad691ca -r 0b215bda525d nashorn/src/jdk/nashorn/internal/codegen/SplitMethodEmitter.java --- 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); diff -r 40665ad691ca -r 0b215bda525d nashorn/src/jdk/nashorn/internal/ir/Block.java --- 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); diff -r 40665ad691ca -r 0b215bda525d nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java --- 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)); diff -r 40665ad691ca -r 0b215bda525d nashorn/src/jdk/nashorn/internal/ir/IdentNode.java --- 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. + * + *

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.

+ * + *
{@code
+     *   try {
+     *     return 2;
+     *   } finally {
+     *     return 3;
+     *   }
+     * }
* * @return true if can have callsite type */ @Override public boolean canHaveCallSiteType() { - return getSymbol() != null && getSymbol().isScope(); + return getSymbol() != null && (getSymbol().isScope() || getSymbol().isNonGenericReturn()); } /** diff -r 40665ad691ca -r 0b215bda525d nashorn/src/jdk/nashorn/internal/ir/Symbol.java --- 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