--- a/nashorn/src/jdk/nashorn/internal/codegen/Attr.java Fri Apr 26 15:13:09 2013 -0300
+++ b/nashorn/src/jdk/nashorn/internal/codegen/Attr.java Mon Apr 29 13:21:17 2013 +0200
@@ -79,6 +79,7 @@
import jdk.nashorn.internal.ir.TryNode;
import jdk.nashorn.internal.ir.UnaryNode;
import jdk.nashorn.internal.ir.VarNode;
+import jdk.nashorn.internal.ir.WithNode;
import jdk.nashorn.internal.ir.visitor.NodeOperatorVisitor;
import jdk.nashorn.internal.ir.visitor.NodeVisitor;
import jdk.nashorn.internal.parser.TokenType;
@@ -495,10 +496,8 @@
}
identNode.setSymbol(symbol);
- // non-local: we need to put symbol in scope (if it isn't already)
- if (!isLocal(lc.getCurrentFunction(), symbol) && !symbol.isScope()) {
- Symbol.setSymbolIsScope(lc, symbol);
- }
+ // if symbol is non-local or we're in a with block, we need to put symbol in scope (if it isn't already)
+ maybeForceScope(symbol);
} else {
LOG.info("No symbol exists. Declare undefined: ", symbol);
symbol = defineSymbol(block, name, IS_GLOBAL, identNode);
@@ -520,6 +519,50 @@
return false;
}
+ /**
+ * If the symbol isn't already a scope symbol, and it is either not local to the current function, or it is being
+ * referenced from within a with block, we force it to be a scope symbol.
+ * @param symbol the symbol that might be scoped
+ */
+ private void maybeForceScope(final Symbol symbol) {
+ if(!symbol.isScope() && symbolNeedsToBeScope(symbol)) {
+ Symbol.setSymbolIsScope(getLexicalContext(), symbol);
+ }
+ }
+
+ private boolean symbolNeedsToBeScope(Symbol symbol) {
+ if(symbol.isThis() || symbol.isInternal()) {
+ return false;
+ }
+ boolean previousWasBlock = false;
+ for(final Iterator<LexicalContextNode> it = getLexicalContext().getAllNodes(); it.hasNext();) {
+ final LexicalContextNode node = it.next();
+ if(node instanceof FunctionNode) {
+ // We reached the function boundary without seeing a definition for the symbol - it needs to be in
+ // scope.
+ return true;
+ } else if(node instanceof WithNode) {
+ if(previousWasBlock) {
+ // We reached a WithNode; the symbol must be scoped. Note that if the WithNode was not immediately
+ // preceded by a block, this means we're currently processing its expression, not its body,
+ // therefore it doesn't count.
+ return true;
+ }
+ previousWasBlock = false;
+ } else if(node instanceof Block) {
+ if(((Block)node).getExistingSymbol(symbol.getName()) == symbol) {
+ // We reached the block that defines the symbol without reaching either the function boundary, or a
+ // WithNode. The symbol need not be scoped.
+ return false;
+ }
+ previousWasBlock = true;
+ } else {
+ previousWasBlock = false;
+ }
+ }
+ throw new AssertionError();
+ }
+
private void setBlockScope(final String name, final Symbol symbol) {
assert symbol != null;
if (symbol.isGlobal()) {
@@ -963,18 +1006,17 @@
final Node lhs = binaryNode.lhs();
if (lhs instanceof IdentNode) {
- final LexicalContext lc = getLexicalContext();
- final Block block = lc.getCurrentBlock();
- final IdentNode ident = (IdentNode)lhs;
- final String name = ident.getName();
+ final Block block = getLexicalContext().getCurrentBlock();
+ final IdentNode ident = (IdentNode)lhs;
+ final String name = ident.getName();
Symbol symbol = findSymbol(block, name);
if (symbol == null) {
symbol = defineSymbol(block, name, IS_GLOBAL, ident);
binaryNode.setSymbol(symbol);
- } else if (!isLocal(lc.getCurrentFunction(), symbol)) {
- Symbol.setSymbolIsScope(lc, symbol);
+ } else {
+ maybeForceScope(symbol);
}
addLocalDef(name);
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Fri Apr 26 15:13:09 2013 -0300
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Apr 29 13:21:17 2013 +0200
@@ -87,6 +87,7 @@
import jdk.nashorn.internal.ir.IfNode;
import jdk.nashorn.internal.ir.IndexNode;
import jdk.nashorn.internal.ir.LexicalContext;
+import jdk.nashorn.internal.ir.LexicalContextNode;
import jdk.nashorn.internal.ir.LineNumberNode;
import jdk.nashorn.internal.ir.LiteralNode;
import jdk.nashorn.internal.ir.LiteralNode.ArrayLiteralNode;
@@ -201,6 +202,7 @@
* @param compiler
*/
CodeGenerator(final Compiler compiler) {
+ super(new DynamicScopeTrackingLexicalContext());
this.compiler = compiler;
this.callSiteFlags = compiler.getEnv()._callsite_flags;
}
@@ -285,23 +287,99 @@
}
/**
+ * A lexical context that also tracks if we have any dynamic scopes in the context. Such scopes can have new
+ * variables introduced into them at run time - a with block or a function directly containing an eval call.
+ */
+ private static class DynamicScopeTrackingLexicalContext extends LexicalContext {
+ int dynamicScopeCount = 0;
+
+ @Override
+ public <T extends LexicalContextNode> T push(T node) {
+ if(isDynamicScopeBoundary(node)) {
+ ++dynamicScopeCount;
+ }
+ return super.push(node);
+ }
+
+ @Override
+ public <T extends LexicalContextNode> T pop(T node) {
+ final T popped = super.pop(node);
+ if(isDynamicScopeBoundary(popped)) {
+ --dynamicScopeCount;
+ }
+ return popped;
+ }
+
+ private boolean isDynamicScopeBoundary(LexicalContextNode node) {
+ if(node instanceof Block) {
+ // Block's immediate parent is a with node. Note we aren't testing for a WithNode, as that'd capture
+ // processing of WithNode.expression too, but it should be unaffected.
+ return !isEmpty() && peek() instanceof WithNode;
+ } else if(node instanceof FunctionNode) {
+ // Function has a direct eval in it (so a top-level "var ..." in the eval code can introduce a new
+ // variable into the function's scope), and it isn't strict (as evals in strict functions get an
+ // isolated scope).
+ return isFunctionDynamicScope((FunctionNode)node);
+ }
+ return false;
+ }
+ }
+
+ boolean inDynamicScope() {
+ return ((DynamicScopeTrackingLexicalContext)getLexicalContext()).dynamicScopeCount > 0;
+ }
+
+ static boolean isFunctionDynamicScope(FunctionNode fn) {
+ return fn.hasEval() && !fn.isStrict();
+ }
+
+ /**
* Check if this symbol can be accessed directly with a putfield or getfield or dynamic load
*
* @param function function to check for fast scope
* @return true if fast scope
*/
private boolean isFastScope(final Symbol symbol) {
- if (!symbol.isScope() || !getLexicalContext().getDefiningBlock(symbol).needsScope()) {
+ if(!symbol.isScope()) {
+ return false;
+ }
+ final LexicalContext lc = getLexicalContext();
+ if(!inDynamicScope()) {
+ // If there's no with or eval in context, and the symbol is marked as scoped, it is fast scoped. Such a
+ // symbol must either be global, or its defining block must need scope.
+ assert symbol.isGlobal() || lc.getDefiningBlock(symbol).needsScope() : symbol.getName();
+ return true;
+ }
+ if(symbol.isGlobal()) {
+ // Shortcut: if there's a with or eval in context, globals can't be fast scoped
return false;
}
- // Allow fast scope access if no function contains with or eval
- for (final Iterator<FunctionNode> it = getLexicalContext().getFunctions(); it.hasNext();) {
- final FunctionNode func = it.next();
- if (func.hasWith() || func.hasEval()) {
- return false;
+ // Otherwise, check if there's a dynamic scope between use of the symbol and its definition
+ final String name = symbol.getName();
+ boolean previousWasBlock = false;
+ for (final Iterator<LexicalContextNode> it = lc.getAllNodes(); it.hasNext();) {
+ final LexicalContextNode node = it.next();
+ if(node instanceof Block) {
+ // If this block defines the symbol, then we can fast scope the symbol.
+ final Block block = (Block)node;
+ if(block.getExistingSymbol(name) == symbol) {
+ assert block.needsScope();
+ return true;
+ }
+ previousWasBlock = true;
+ } else {
+ if((node instanceof WithNode && previousWasBlock) || (node instanceof FunctionNode && isFunctionDynamicScope((FunctionNode)node))) {
+ // If we hit a scope that can have symbols introduced into it at run time before finding the defining
+ // block, the symbol can't be fast scoped. A WithNode only counts if we've immediately seen a block
+ // before - its block. Otherwise, we are currently processing the WithNode's expression, and that's
+ // obviously not subjected to introducing new symbols.
+ return false;
+ }
+ previousWasBlock = false;
}
}
- return true;
+ // Should've found the symbol defined in a block
+ throw new AssertionError();
}
private MethodEmitter loadSharedScopeVar(final Type valueType, final Symbol symbol, final int flags) {
@@ -671,7 +749,7 @@
evalCall(node, flags);
} else if (useCount <= SharedScopeCall.FAST_SCOPE_CALL_THRESHOLD
|| (!isFastScope(symbol) && useCount <= SharedScopeCall.SLOW_SCOPE_CALL_THRESHOLD)
- || callNode.inWithBlock()) {
+ || CodeGenerator.this.inDynamicScope()) {
scopeCall(node, flags);
} else {
sharedScopeCall(node, flags);
@@ -2114,9 +2192,11 @@
}
private void closeWith() {
- method.loadCompilerConstant(SCOPE);
- method.invoke(ScriptRuntime.CLOSE_WITH);
- method.storeCompilerConstant(SCOPE);
+ if(method.hasScope()) {
+ method.loadCompilerConstant(SCOPE);
+ method.invoke(ScriptRuntime.CLOSE_WITH);
+ method.storeCompilerConstant(SCOPE);
+ }
}
@Override
@@ -2124,38 +2204,58 @@
final Node expression = withNode.getExpression();
final Node body = withNode.getBody();
- final Label tryLabel = new Label("with_try");
- final Label endLabel = new Label("with_end");
- final Label catchLabel = new Label("with_catch");
- final Label exitLabel = new Label("with_exit");
-
- method.label(tryLabel);
-
- method.loadCompilerConstant(SCOPE);
+ // It is possible to have a "pathological" case where the with block does not reference *any* identifiers. It's
+ // pointless, but legal. In that case, if nothing else in the method forced the assignment of a slot to the
+ // scope object, its' possible that it won't have a slot assigned. In this case we'll only evaluate expression
+ // for its side effect and visit the body, and not bother opening and closing a WithObject.
+ final boolean hasScope = method.hasScope();
+
+ final Label tryLabel;
+ if(hasScope) {
+ tryLabel = new Label("with_try");
+ method.label(tryLabel);
+ method.loadCompilerConstant(SCOPE);
+ } else {
+ tryLabel = null;
+ }
+
load(expression);
-
assert expression.getType().isObject() : "with expression needs to be object: " + expression;
- method.invoke(ScriptRuntime.OPEN_WITH);
- method.storeCompilerConstant(SCOPE);
-
+ if(hasScope) {
+ // Construct a WithObject if we have a scope
+ method.invoke(ScriptRuntime.OPEN_WITH);
+ method.storeCompilerConstant(SCOPE);
+ } else {
+ // We just loaded the expression for its side effect; discard it
+ method.pop();
+ }
+
+
+ // Always process body
body.accept(this);
- if (!body.isTerminal()) {
+ if(hasScope) {
+ // Ensure we always close the WithObject
+ final Label endLabel = new Label("with_end");
+ final Label catchLabel = new Label("with_catch");
+ final Label exitLabel = new Label("with_exit");
+
+ if (!body.isTerminal()) {
+ closeWith();
+ method._goto(exitLabel);
+ }
+
+ method.label(endLabel);
+
+ method._catch(catchLabel);
closeWith();
- method._goto(exitLabel);
+ method.athrow();
+
+ method.label(exitLabel);
+
+ method._try(tryLabel, endLabel, catchLabel);
}
-
- method.label(endLabel);
-
- method._catch(catchLabel);
- closeWith();
- method.athrow();
-
- method.label(exitLabel);
-
- method._try(tryLabel, endLabel, catchLabel);
-
return false;
}
--- a/nashorn/src/jdk/nashorn/internal/codegen/MethodEmitter.java Fri Apr 26 15:13:09 2013 -0300
+++ b/nashorn/src/jdk/nashorn/internal/codegen/MethodEmitter.java Mon Apr 29 13:21:17 2013 +0200
@@ -818,7 +818,7 @@
}
/**
- * Push an local variable to the stack. If the symbol representing
+ * Push a local variable to the stack. If the symbol representing
* the local variable doesn't have a slot, this is a NOP
*
* @param symbol the symbol representing the local variable.
@@ -900,6 +900,15 @@
return functionNode.getBody().getExistingSymbol(cc.symbolName());
}
+ /**
+ * True if this method has a slot allocated for the scope variable (meaning, something in the method actually needs
+ * the scope).
+ * @return if this method has a slot allocated for the scope variable.
+ */
+ boolean hasScope() {
+ return compilerConstant(SCOPE).hasSlot();
+ }
+
MethodEmitter loadCompilerConstant(final CompilerConstants cc) {
final Symbol symbol = compilerConstant(cc);
if (cc == SCOPE && peekType() == Type.SCOPE) {
--- a/nashorn/src/jdk/nashorn/internal/ir/CallNode.java Fri Apr 26 15:13:09 2013 -0300
+++ b/nashorn/src/jdk/nashorn/internal/ir/CallNode.java Mon Apr 29 13:21:17 2013 +0200
@@ -50,9 +50,6 @@
/** Is this a "new" operation */
public static final int IS_NEW = 0x1;
- /** Is this call tagged as inside a with block */
- public static final int IN_WITH_BLOCK = 0x2;
-
private final int flags;
/**
@@ -145,14 +142,13 @@
* @param finish finish
* @param function the function to call
* @param args args to the call
- * @param flags flags
*/
- public CallNode(final Source source, final long token, final int finish, final Node function, final List<Node> args, final int flags) {
+ public CallNode(final Source source, final long token, final int finish, final Node function, final List<Node> args) {
super(source, token, finish);
this.function = function;
this.args = args;
- this.flags = flags;
+ this.flags = 0;
this.type = null;
this.evalArgs = null;
}
@@ -333,14 +329,6 @@
return setFlags(IS_NEW);
}
- /**
- * Check if this call is inside a {@code with} block
- * @return true if the call is inside a {@code with} block
- */
- public boolean inWithBlock() {
- return (flags & IN_WITH_BLOCK) == IN_WITH_BLOCK;
- }
-
private CallNode setFlags(final int flags) {
if (this.flags == flags) {
return this;
--- a/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java Fri Apr 26 15:13:09 2013 -0300
+++ b/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java Mon Apr 29 13:21:17 2013 +0200
@@ -145,14 +145,12 @@
/** Has this node been split because it was too large? */
public static final int IS_SPLIT = 1 << 4;
- /** Does the function call eval? */
+ /** Does the function call eval? If it does, then all variables in this function might be get/set by it and it can
+ * introduce new variables into this function's scope too.*/
public static final int HAS_EVAL = 1 << 5;
- /** Does the function contain a with block ? */
- public static final int HAS_WITH = 1 << 6;
-
- /** Does a descendant function contain a with or eval? */
- public static final int HAS_DESCENDANT_WITH_OR_EVAL = 1 << 7;
+ /** Does a nested function contain eval? If it does, then all variables in this function might be get/set by it. */
+ public static final int HAS_NESTED_EVAL = 1 << 6;
/**
* Flag this function as one that defines the identifier "arguments" as a function parameter or nested function
@@ -178,18 +176,18 @@
/** Does this function have nested declarations? */
public static final int HAS_FUNCTION_DECLARATIONS = 1 << 13;
- /** Does this function or any nested functions contain a with or an eval? */
- private static final int HAS_DEEP_WITH_OR_EVAL = HAS_EVAL | HAS_WITH | HAS_DESCENDANT_WITH_OR_EVAL;
+ /** Does this function or any nested functions contain an eval? */
+ private static final int HAS_DEEP_EVAL = HAS_EVAL | HAS_NESTED_EVAL;
/** Does this function need to store all its variables in scope? */
- private static final int HAS_ALL_VARS_IN_SCOPE = HAS_DEEP_WITH_OR_EVAL | IS_SPLIT | HAS_LAZY_CHILDREN;
+ private static final int HAS_ALL_VARS_IN_SCOPE = HAS_DEEP_EVAL | IS_SPLIT | HAS_LAZY_CHILDREN;
/** Does this function potentially need "arguments"? Note that this is not a full test, as further negative check of REDEFINES_ARGS is needed. */
private static final int MAYBE_NEEDS_ARGUMENTS = USES_ARGUMENTS | HAS_EVAL;
- /** Does this function need the parent scope? It needs it if either it or its descendants use variables from it, or have a deep with or eval.
+ /** Does this function need the parent scope? It needs it if either it or its descendants use variables from it, or have a deep eval.
* We also pessimistically need a parent scope if we have lazy children that have not yet been compiled */
- private static final int NEEDS_PARENT_SCOPE = USES_ANCESTOR_SCOPE | HAS_DEEP_WITH_OR_EVAL | HAS_LAZY_CHILDREN;
+ private static final int NEEDS_PARENT_SCOPE = USES_ANCESTOR_SCOPE | HAS_DEEP_EVAL | HAS_LAZY_CHILDREN;
/** What is the return type of this function? */
private Type returnType = Type.UNKNOWN;
@@ -406,15 +404,6 @@
}
/**
- * Check if the {@code with} keyword is used in this function
- *
- * @return true if {@code with} is used
- */
- public boolean hasWith() {
- return getFlag(HAS_WITH);
- }
-
- /**
* Check if the {@code eval} keyword is used in this function
*
* @return true if {@code eval} is used
@@ -424,18 +413,6 @@
}
/**
- * Test whether this function or any of its nested functions contains a <tt>with</tt> statement
- * or an <tt>eval</tt> call.
- *
- * @see #hasWith()
- * @see #hasEval()
- * @return true if this or a nested function contains with or eval
- */
- public boolean hasDeepWithOrEval() {
- return getFlag(HAS_DEEP_WITH_OR_EVAL);
- }
-
- /**
* Get the first token for this function
* @return the first token
*/
--- a/nashorn/src/jdk/nashorn/internal/ir/LexicalContext.java Fri Apr 26 15:13:09 2013 -0300
+++ b/nashorn/src/jdk/nashorn/internal/ir/LexicalContext.java Mon Apr 29 13:21:17 2013 +0200
@@ -420,14 +420,6 @@
}
/**
- * Check if lexical context is currently inside a with block
- * @return true if in a with block
- */
- public boolean inWith() {
- return getScopeNestingLevelTo(null) > 0;
- }
-
- /**
* Count the number of with scopes until a given node
* @param until node to stop counting at, or null if all nodes should be counted
* @return number of with scopes encountered in the context
--- a/nashorn/src/jdk/nashorn/internal/parser/Parser.java Fri Apr 26 15:13:09 2013 -0300
+++ b/nashorn/src/jdk/nashorn/internal/parser/Parser.java Mon Apr 29 13:21:17 2013 +0200
@@ -398,7 +398,7 @@
final String name = ident.getName();
if (EVAL.symbolName().equals(name)) {
- markWithOrEval(lc, FunctionNode.HAS_EVAL);
+ markEval(lc);
}
}
@@ -1353,7 +1353,6 @@
// Get WITH expression.
WithNode withNode = new WithNode(source, withToken, finish);
- markWithOrEval(lc, FunctionNode.HAS_WITH);
try {
lc.push(withNode);
@@ -1742,7 +1741,7 @@
// Skip ending of edit string expression.
expect(RBRACE);
- return new CallNode(source, primaryToken, finish, execIdent, arguments, 0);
+ return new CallNode(source, primaryToken, finish, execIdent, arguments);
}
/**
@@ -2041,10 +2040,6 @@
return new PropertyNode(source, propertyToken, finish, propertyName, assignmentExpression(false), null, null);
}
- private int callNodeFlags() {
- return lc.inWith() ? CallNode.IN_WITH_BLOCK : 0;
- }
-
/**
* LeftHandSideExpression :
* NewExpression
@@ -2074,7 +2069,7 @@
detectSpecialFunction((IdentNode)lhs);
}
- lhs = new CallNode(source, callToken, finish, lhs, arguments, callNodeFlags());
+ lhs = new CallNode(source, callToken, finish, lhs, arguments);
}
loop:
@@ -2088,7 +2083,7 @@
final List<Node> arguments = argumentList();
// Create call node.
- lhs = new CallNode(source, callToken, finish, lhs, arguments, callNodeFlags());
+ lhs = new CallNode(source, callToken, finish, lhs, arguments);
break;
@@ -2167,7 +2162,7 @@
arguments.add(objectLiteral());
}
- final CallNode callNode = new CallNode(source, constructor.getToken(), finish, constructor, arguments, callNodeFlags());
+ final CallNode callNode = new CallNode(source, constructor.getToken(), finish, constructor, arguments);
return new UnaryNode(source, newToken, callNode);
}
@@ -2822,16 +2817,16 @@
return "[JavaScript Parsing]";
}
- private static void markWithOrEval(final LexicalContext lc, int flag) {
+ private static void markEval(final LexicalContext lc) {
final Iterator<FunctionNode> iter = lc.getFunctions();
boolean flaggedCurrentFn = false;
while (iter.hasNext()) {
final FunctionNode fn = iter.next();
if (!flaggedCurrentFn) {
- lc.setFlag(fn, flag);
+ lc.setFlag(fn, FunctionNode.HAS_EVAL);
flaggedCurrentFn = true;
} else {
- lc.setFlag(fn, FunctionNode.HAS_DESCENDANT_WITH_OR_EVAL);
+ lc.setFlag(fn, FunctionNode.HAS_NESTED_EVAL);
}
lc.setFlag(lc.getFunctionBody(fn), Block.NEEDS_SCOPE);
}
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/NashornCallSiteDescriptor.java Fri Apr 26 15:13:09 2013 -0300
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/NashornCallSiteDescriptor.java Mon Apr 29 13:21:17 2013 +0200
@@ -43,9 +43,8 @@
public static final int CALLSITE_SCOPE = 0x01;
/** Flags that the call site is in code that uses ECMAScript strict mode. */
public static final int CALLSITE_STRICT = 0x02;
- /** Flags that a property getter or setter call site references a scope variable that is not in the global scope
- * (it is in a function lexical scope), and the function's scope object class is fixed and known in advance. Such
- * getters and setters can often be linked more optimally using these assumptions. */
+ /** Flags that a property getter or setter call site references a scope variable that is located at a known distance
+ * in the scope chain. Such getters and setters can often be linked more optimally using these assumptions. */
public static final int CALLSITE_FAST_SCOPE = 0x400;
/** Flags that the call site is profiled; Contexts that have {@code "profile.callsites"} boolean property set emit