Mercurial > hg > openjdk > jdk8 > nashorn

changeset 415:ce9cbe70f915

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
8019819: scope symbol didn't get a slot in certain cases Reviewed-by: hannesw, jlaskey, lagergren, sundar
author attila
date Fri, 05 Jul 2013 15:10:47 +0200
parents edca88d3a03e
children 20b2c2dc20e8
files src/jdk/nashorn/internal/codegen/Attr.java src/jdk/nashorn/internal/codegen/CodeGenerator.java src/jdk/nashorn/internal/codegen/CompilationPhase.java src/jdk/nashorn/internal/codegen/FinalizeTypes.java src/jdk/nashorn/internal/ir/FunctionNode.java src/jdk/nashorn/internal/ir/LexicalContext.java src/jdk/nashorn/internal/ir/Symbol.java src/jdk/nashorn/internal/parser/Parser.java test/script/basic/JDK-8019819.js
diffstat 9 files changed, 108 insertions(+), 37 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk/nashorn/internal/codegen/Attr.java	Fri Jul 05 14:36:54 2013 +0200
+++ b/src/jdk/nashorn/internal/codegen/Attr.java	Fri Jul 05 15:10:47 2013 +0200
@@ -54,7 +54,6 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
-
 import jdk.nashorn.internal.codegen.types.Type;
 import jdk.nashorn.internal.ir.AccessNode;
 import jdk.nashorn.internal.ir.BinaryNode;
@@ -343,10 +342,11 @@
         catchNestingLevel++;
 
         // define block-local exception variable
-        final Symbol def = defineSymbol(block, exception.getName(), IS_VAR | IS_LET | IS_ALWAYS_DEFINED);
+        final String exname = exception.getName();
+        final Symbol def = defineSymbol(block, exname, IS_VAR | IS_LET | IS_ALWAYS_DEFINED);
         newType(def, Type.OBJECT); //we can catch anything, not just ecma exceptions
 
-        addLocalDef(exception.getName());
+        addLocalDef(exname);
 
         return true;
     }
@@ -678,7 +678,7 @@
 
             if (scopeBlock != null) {
                 assert lc.contains(scopeBlock);
-                lc.setFlag(scopeBlock, Block.NEEDS_SCOPE);
+                lc.setBlockNeedsScope(scopeBlock);
             }
         }
     }
--- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Fri Jul 05 14:36:54 2013 +0200
+++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Fri Jul 05 15:10:47 2013 +0200
@@ -881,9 +881,15 @@
 
         final FunctionNode function = lc.getCurrentFunction();
         if (isFunctionBody) {
-            /* Fix the predefined slots so they have numbers >= 0, like varargs. */
-            if (function.needsParentScope()) {
-                initParentScope();
+            if(method.hasScope()) {
+                if (function.needsParentScope()) {
+                    method.loadCompilerConstant(CALLEE);
+                    method.invoke(ScriptFunction.GET_SCOPE);
+                } else {
+                    assert function.hasScopeBlock();
+                    method.loadNull();
+                }
+                method.storeCompilerConstant(SCOPE);
             }
             if (function.needsArguments()) {
                 initArguments(function);
@@ -949,15 +955,6 @@
                 protected void loadValue(final Symbol value) {
                     method.load(value);
                 }
-
-                @Override
-                protected void loadScope(MethodEmitter m) {
-                    if (function.needsParentScope()) {
-                        m.loadCompilerConstant(SCOPE);
-                    } else {
-                        m.loadNull();
-                    }
-                }
             }.makeObject(method);
 
             // runScript(): merge scope into global
@@ -998,12 +995,6 @@
         method.storeCompilerConstant(ARGUMENTS);
     }
 
-    private void initParentScope() {
-        method.loadCompilerConstant(CALLEE);
-        method.invoke(ScriptFunction.GET_SCOPE);
-        method.storeCompilerConstant(SCOPE);
-    }
-
     @Override
     public boolean enterFunctionNode(final FunctionNode functionNode) {
         if (functionNode.isLazy()) {
--- a/src/jdk/nashorn/internal/codegen/CompilationPhase.java	Fri Jul 05 14:36:54 2013 +0200
+++ b/src/jdk/nashorn/internal/codegen/CompilationPhase.java	Fri Jul 05 15:10:47 2013 +0200
@@ -18,17 +18,15 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-
 import jdk.nashorn.internal.codegen.types.Range;
 import jdk.nashorn.internal.codegen.types.Type;
-import jdk.nashorn.internal.ir.Block;
 import jdk.nashorn.internal.ir.CallNode;
 import jdk.nashorn.internal.ir.FunctionNode;
+import jdk.nashorn.internal.ir.FunctionNode.CompilationState;
 import jdk.nashorn.internal.ir.LexicalContext;
+import jdk.nashorn.internal.ir.Node;
 import jdk.nashorn.internal.ir.ReturnNode;
 import jdk.nashorn.internal.ir.Symbol;
-import jdk.nashorn.internal.ir.FunctionNode.CompilationState;
-import jdk.nashorn.internal.ir.Node;
 import jdk.nashorn.internal.ir.TemporarySymbols;
 import jdk.nashorn.internal.ir.debug.ASTWriter;
 import jdk.nashorn.internal.ir.debug.PrintVisitor;
@@ -117,7 +115,7 @@
                         final FunctionNode parent = lc.getParentFunction(functionNode);
                         assert parent != null;
                         lc.setFlag(parent, FunctionNode.HAS_LAZY_CHILDREN);
-                        lc.setFlag(parent.getBody(), Block.NEEDS_SCOPE);
+                        lc.setBlockNeedsScope(parent.getBody());
                         lc.setFlag(functionNode, FunctionNode.IS_LAZY);
                         return functionNode;
                     }
--- a/src/jdk/nashorn/internal/codegen/FinalizeTypes.java	Fri Jul 05 14:36:54 2013 +0200
+++ b/src/jdk/nashorn/internal/codegen/FinalizeTypes.java	Fri Jul 05 15:10:47 2013 +0200
@@ -31,7 +31,6 @@
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
-
 import jdk.nashorn.internal.codegen.types.Type;
 import jdk.nashorn.internal.ir.AccessNode;
 import jdk.nashorn.internal.ir.Assignment;
@@ -415,10 +414,10 @@
         if (!functionNode.needsCallee()) {
             functionNode.compilerConstant(CALLEE).setNeedsSlot(false);
         }
-        // Similar reasoning applies to __scope__ symbol: if the function doesn't need either parent scope or its
-        // own scope, we ensure it doesn't get a slot, but we can't determine whether it needs a scope earlier than
-        // this phase.
-        if (!(functionNode.getBody().needsScope() || functionNode.needsParentScope())) {
+        // Similar reasoning applies to __scope__ symbol: if the function doesn't need either parent scope and none of
+        // its blocks create a scope, we ensure it doesn't get a slot, but we can't determine whether it needs a scope
+        // earlier than this phase.
+        if (!(functionNode.hasScopeBlock() || functionNode.needsParentScope())) {
             functionNode.compilerConstant(SCOPE).setNeedsSlot(false);
         }
 
--- a/src/jdk/nashorn/internal/ir/FunctionNode.java	Fri Jul 05 14:36:54 2013 +0200
+++ b/src/jdk/nashorn/internal/ir/FunctionNode.java	Fri Jul 05 15:10:47 2013 +0200
@@ -160,6 +160,10 @@
     /** 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;
 
+    /** Does this function have any blocks that create a scope? This is used to determine if the function needs to
+     * have a local variable slot for the scope symbol. */
+    public static final int HAS_SCOPE_BLOCK = 1 << 7;
+
     /**
      * Flag this function as one that defines the identifier "arguments" as a function parameter or nested function
      * name. This precludes it from needing to have an Arguments object defined as "arguments" local variable. Note that
@@ -577,7 +581,7 @@
         if(this.body == body) {
             return this;
         }
-        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags | (body.needsScope() ? FunctionNode.HAS_SCOPE_BLOCK : 0), name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
     }
 
     /**
@@ -638,6 +642,14 @@
     }
 
     /**
+     * Returns true if any of the blocks in this function create their own scope.
+     * @return true if any of the blocks in this function create their own scope.
+     */
+    public boolean hasScopeBlock() {
+        return getFlag(HAS_SCOPE_BLOCK);
+    }
+
+    /**
      * Return the kind of this function
      * @see FunctionNode.Kind
      * @return the kind
--- a/src/jdk/nashorn/internal/ir/LexicalContext.java	Fri Jul 05 14:36:54 2013 +0200
+++ b/src/jdk/nashorn/internal/ir/LexicalContext.java	Fri Jul 05 15:10:47 2013 +0200
@@ -54,13 +54,16 @@
 
     /**
      * Set the flags for a lexical context node on the stack. Does not
-     * replace the flags, but rather adds to them
+     * replace the flags, but rather adds to them.
      *
      * @param node  node
      * @param flag  new flag to set
      */
     public void setFlag(final LexicalContextNode node, final int flag) {
         if (flag != 0) {
+            // Use setBlockNeedsScope() instead
+            assert !(flag == Block.NEEDS_SCOPE && node instanceof Block);
+
             for (int i = sp - 1; i >= 0; i--) {
                 if (stack[i] == node) {
                     flags[i] |= flag;
@@ -72,6 +75,29 @@
     }
 
     /**
+     * Marks the block as one that creates a scope. Note that this method must
+     * be used instead of {@link #setFlag(LexicalContextNode, int)} with
+     * {@link Block#NEEDS_SCOPE} because it atomically also sets the
+     * {@link FunctionNode#HAS_SCOPE_BLOCK} flag on the block's containing
+     * function.
+     * @param block the block that needs to be marked as creating a scope.
+     */
+    public void setBlockNeedsScope(final Block block) {
+        for (int i = sp - 1; i >= 0; i--) {
+            if (stack[i] == block) {
+                flags[i] |= Block.NEEDS_SCOPE;
+                for(int j = i - 1; j >=0; j --) {
+                    if(stack[j] instanceof FunctionNode) {
+                        flags[j] |= FunctionNode.HAS_SCOPE_BLOCK;
+                        return;
+                    }
+                }
+            }
+        }
+        assert false;
+    }
+
+    /**
      * Get the flags for a lexical context node on the stack
      * @param node node
      * @return the flags for the node
--- a/src/jdk/nashorn/internal/ir/Symbol.java	Fri Jul 05 14:36:54 2013 +0200
+++ b/src/jdk/nashorn/internal/ir/Symbol.java	Fri Jul 05 15:10:47 2013 +0200
@@ -29,7 +29,6 @@
 import java.util.HashSet;
 import java.util.Set;
 import java.util.StringTokenizer;
-
 import jdk.nashorn.internal.codegen.types.Range;
 import jdk.nashorn.internal.codegen.types.Type;
 import jdk.nashorn.internal.runtime.Context;
@@ -705,7 +704,7 @@
     public static void setSymbolIsScope(final LexicalContext lc, final Symbol symbol) {
         symbol.setIsScope();
         if (!symbol.isGlobal()) {
-            lc.setFlag(lc.getDefiningBlock(symbol), Block.NEEDS_SCOPE);
+            lc.setBlockNeedsScope(lc.getDefiningBlock(symbol));
         }
     }
 
--- a/src/jdk/nashorn/internal/parser/Parser.java	Fri Jul 05 14:36:54 2013 +0200
+++ b/src/jdk/nashorn/internal/parser/Parser.java	Fri Jul 05 15:10:47 2013 +0200
@@ -2957,7 +2957,7 @@
             } else {
                 lc.setFlag(fn, FunctionNode.HAS_NESTED_EVAL);
             }
-            lc.setFlag(lc.getFunctionBody(fn), Block.NEEDS_SCOPE);
+            lc.setBlockNeedsScope(lc.getFunctionBody(fn));
         }
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/script/basic/JDK-8019819.js	Fri Jul 05 15:10:47 2013 +0200
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8019819: scope symbol didn't get a slot in certain cases
+ * 
+ * @test
+ * @run
+ */
+function f() {
+    try {
+    } catch(e if [].g(e)) { 
+        with({}) { 
+            throw e;
+        }
+    }
+}
+
+function g() {
+    try {
+    } catch(e) { 
+        with({}) { 
+            throw e;
+        }
+    }
+}