8013919: Original exception no longer thrown away when a finally rethrows
authorlagergren
Thu, 16 May 2013 13:44:25 +0200
changeset 17752 9d2d0e74a833
parent 17751 15a93d511eb7
child 17753 4800f03fbfc4
8013919: Original exception no longer thrown away when a finally rethrows Reviewed-by: jlaskey, sundar
nashorn/src/jdk/nashorn/internal/codegen/Lower.java
nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java
nashorn/test/script/basic/JDK-8013919.js
nashorn/test/script/basic/JDK-8013919.js.EXPECTED
--- a/nashorn/src/jdk/nashorn/internal/codegen/Lower.java	Thu May 16 14:52:48 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/codegen/Lower.java	Thu May 16 13:44:25 2013 +0200
@@ -261,19 +261,25 @@
         return throwNode;
     }
 
-    private static Node ensureUniqueLabelsIn(final Node node) {
+    private static Node ensureUniqueNamesIn(final LexicalContext lc, final Node node) {
         return node.accept(new NodeVisitor() {
-           @Override
-           public Node leaveDefault(final Node labelledNode) {
-               return labelledNode.ensureUniqueLabels(getLexicalContext());
-           }
+            @Override
+            public Node leaveFunctionNode(final FunctionNode functionNode) {
+                final String name = functionNode.getName();
+                return functionNode.setName(getLexicalContext(), lc.getCurrentFunction().uniqueName(name));
+            }
+
+            @Override
+            public Node leaveDefault(final Node labelledNode) {
+                return labelledNode.ensureUniqueLabels(getLexicalContext());
+            }
         });
     }
 
-    private static List<Statement> copyFinally(final Block finallyBody) {
+    private static List<Statement> copyFinally(final LexicalContext lc, final Block finallyBody) {
         final List<Statement> newStatements = new ArrayList<>();
         for (final Statement statement : finallyBody.getStatements()) {
-            newStatements.add((Statement)ensureUniqueLabelsIn(statement));
+            newStatements.add((Statement)ensureUniqueNamesIn(lc, statement));
             if (statement.hasTerminalFlags()) {
                 return newStatements;
             }
@@ -316,9 +322,9 @@
      * @return new try node after splicing finally code (same if nop)
      */
     private Node spliceFinally(final TryNode tryNode, final List<ThrowNode> rethrows, final Block finallyBody) {
-        final int finish = tryNode.getFinish();
-
         assert tryNode.getFinallyBody() == null;
+        final int            finish = tryNode.getFinish();
+        final LexicalContext lc     = getLexicalContext();
 
         final TryNode newTryNode = (TryNode)tryNode.accept(new NodeVisitor() {
             final List<Node> insideTry = new ArrayList<>();
@@ -338,7 +344,7 @@
             @Override
             public Node leaveThrowNode(final ThrowNode throwNode) {
                 if (rethrows.contains(throwNode)) {
-                    final List<Statement> newStatements = copyFinally(finallyBody);
+                    final List<Statement> newStatements = copyFinally(lc, finallyBody);
                     if (!isTerminal(newStatements)) {
                         newStatements.add(throwNode);
                     }
@@ -372,7 +378,7 @@
                     resultNode = null;
                 }
 
-                newStatements.addAll(copyFinally(finallyBody));
+                newStatements.addAll(copyFinally(lc, finallyBody));
                 if (!isTerminal(newStatements)) {
                     newStatements.add(expr == null ? returnNode : returnNode.setExpression(resultNode));
                 }
@@ -382,7 +388,7 @@
 
             private Node copy(final Statement endpoint, final Node targetNode) {
                 if (!insideTry.contains(targetNode)) {
-                    final List<Statement> newStatements = copyFinally(finallyBody);
+                    final List<Statement> newStatements = copyFinally(lc, finallyBody);
                     if (!isTerminal(newStatements)) {
                         newStatements.add(endpoint);
                     }
@@ -548,7 +554,7 @@
                 final FunctionNode currentFunction = getLexicalContext().getCurrentFunction();
                 return callNode.setEvalArgs(
                     new CallNode.EvalArgs(
-                        ensureUniqueLabelsIn(args.get(0)).accept(this),
+                        ensureUniqueNamesIn(getLexicalContext(), args.get(0)).accept(this),
                         compilerConstant(THIS),
                         evalLocation(callee),
                         currentFunction.isStrict()));
--- a/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java	Thu May 16 14:52:48 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java	Thu May 16 13:44:25 2013 +0200
@@ -250,6 +250,7 @@
         final FunctionNode functionNode,
         final long lastToken,
         final int flags,
+        final String name,
         final Type returnType,
         final CompileUnit compileUnit,
         final EnumSet<CompilationState> compilationState,
@@ -260,6 +261,7 @@
         super(functionNode);
 
         this.flags            = flags;
+        this.name             = name;
         this.returnType       = returnType;
         this.compileUnit      = compileUnit;
         this.lastToken        = lastToken;
@@ -271,7 +273,6 @@
 
         // the fields below never change - they are final and assigned in constructor
         this.source          = functionNode.source;
-        this.name            = functionNode.name;
         this.ident           = functionNode.ident;
         this.namespace       = functionNode.namespace;
         this.declaredSymbols = functionNode.declaredSymbols;
@@ -315,7 +316,7 @@
         if (this.snapshot == null) {
             return this;
         }
-        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, returnType, compileUnit, compilationState, body, parameters, null, hints));
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, null, hints));
     }
 
     /**
@@ -331,7 +332,7 @@
         if (isProgram() || parameters.isEmpty()) {
             return this; //never specialize anything that won't be recompiled
         }
-        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, returnType, compileUnit, compilationState, body, parameters, this, hints));
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, this, hints));
     }
 
     /**
@@ -389,7 +390,7 @@
         }
         final EnumSet<CompilationState> newState = EnumSet.copyOf(this.compilationState);
         newState.add(state);
-        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, returnType, compileUnit, newState, body, parameters, snapshot, hints));
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, newState, body, parameters, snapshot, hints));
     }
 
     /**
@@ -410,7 +411,7 @@
         if (this.hints == hints) {
             return this;
         }
-        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
     }
 
     /**
@@ -463,7 +464,7 @@
         if (this.flags == flags) {
             return this;
         }
-        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
     }
 
     @Override
@@ -529,7 +530,7 @@
     }
 
     /**
-     * Get the identifier for this function
+     * Get the identifier for this function, this is its symbol.
      * @return the identifier as an IdentityNode
      */
     public IdentNode getIdent() {
@@ -572,7 +573,7 @@
         if(this.body == body) {
             return this;
         }
-        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
     }
 
     /**
@@ -640,7 +641,7 @@
         if (this.lastToken == lastToken) {
             return this;
         }
-        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
     }
 
     /**
@@ -651,6 +652,20 @@
         return name;
     }
 
+
+    /**
+     * Set the internal name for this function
+     * @param lc    lexical context
+     * @param name new name
+     * @return new function node if changed, otherwise the same
+     */
+    public FunctionNode setName(final LexicalContext lc, final String name) {
+        if (this.name.equals(name)) {
+            return this;
+        }
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
+    }
+
     /**
      * Check if this function should have all its variables in its own scope. Scripts, split sub-functions, and
      * functions having with and/or eval blocks are such.
@@ -698,7 +713,7 @@
         if (this.parameters == parameters) {
             return this;
         }
-        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
     }
 
     /**
@@ -762,6 +777,7 @@
                 this,
                 lastToken,
                 flags,
+                name,
                 Type.widest(this.returnType, returnType.isObject() ?
                     Type.OBJECT :
                     returnType),
@@ -801,7 +817,7 @@
         if (this.compileUnit == compileUnit) {
             return this;
         }
-        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
+        return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
     }
 
     /**
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8013919.js	Thu May 16 13:44:25 2013 +0200
@@ -0,0 +1,39 @@
+/*
+ * 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-8013913: finally cloning of function node declarations caused
+ * method collissions
+ *
+ * @test
+ * @run
+ */
+
+try {
+    print("a");
+} finally {
+    var b = function() {
+	print("b");
+    }
+    b();
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8013919.js.EXPECTED	Thu May 16 13:44:25 2013 +0200
@@ -0,0 +1,2 @@
+a
+b