8020718: RETURN symbol has wrong type in split functions
Reviewed-by: lagergren, attila
--- 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