8013325: function named 'arguments' should set DEFINES_ARGUMENTS flag in its parent, not itself

Reviewed-by: hannesw, sundar

nashorn/src/jdk/internal/dynalink/beans/StaticClassIntrospector.java

@@ -84,10 +84,10 @@
 package jdk.internal.dynalink.beans;
 
 import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.util.HashMap;
 import java.util.Map;
-import jdk.nashorn.internal.lookup.Lookup;
 
 class StaticClassIntrospector extends FacetIntrospector {
     StaticClassIntrospector(Class<?> clazz) {
@@ -98,7 +98,7 @@ class StaticClassIntrospector extends FacetIntrospector {
     Map<String, MethodHandle> getInnerClassGetters() {
         final Map<String, MethodHandle> map = new HashMap<>();
         for(Class<?> innerClass: membersLookup.getInnerClasses()) {
-            map.put(innerClass.getSimpleName(), editMethodHandle(Lookup.MH.constant(StaticClass.class,
+            map.put(innerClass.getSimpleName(), editMethodHandle(MethodHandles.constant(StaticClass.class,
                     StaticClass.forClass(innerClass))));
         }
         return map;
@@ -110,7 +110,7 @@ class StaticClassIntrospector extends FacetIntrospector {
     }
 
     static MethodHandle dropReceiver(final MethodHandle mh, final Class<?> receiverClass) {
-        MethodHandle newHandle = Lookup.MH.dropArguments(mh, 0, receiverClass);
+        MethodHandle newHandle = MethodHandles.dropArguments(mh, 0, receiverClass);
         // NOTE: this is a workaround for the fact that dropArguments doesn't preserve vararg collector state.
         if(mh.isVarargsCollector() && !newHandle.isVarargsCollector()) {
             final MethodType type = mh.type();

nashorn/src/jdk/nashorn/internal/codegen/Attr.java

@@ -48,10 +48,8 @@ import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Deque;
 import java.util.HashSet;
-import java.util.IdentityHashMap;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import jdk.nashorn.internal.codegen.types.Type;
 import jdk.nashorn.internal.ir.AccessNode;
@@ -91,6 +89,7 @@ import jdk.nashorn.internal.runtime.ECMAException;
 import jdk.nashorn.internal.runtime.JSType;
 import jdk.nashorn.internal.runtime.Property;
 import jdk.nashorn.internal.runtime.PropertyMap;
+import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptObject;
 
 /**
@@ -126,8 +125,6 @@ final class Attr extends NodeOperatorVisitor {
 
     private final Deque<Type> returnTypes;
 
-    private final Map<Symbol, FunctionNode> selfSymbolToFunction = new IdentityHashMap<>();
-
     private static final DebugLogger LOG   = new DebugLogger("attr");
     private static final boolean     DEBUG = LOG.isEnabled();
 
@@ -173,23 +170,26 @@ final class Attr extends NodeOperatorVisitor {
 
         if (functionNode.isProgram()) {
             initFromPropertyMap(body);
-        }
+        } else if(!functionNode.isDeclared()) {
+            // It's neither declared nor program - it's a function expression then; assign it a self-symbol.
 
-        // Add function name as local symbol
-        if (!functionNode.isDeclared() && !functionNode.isProgram()) {
             if (functionNode.getSymbol() != null) {
                 // a temporary left over from an earlier pass when the function was lazy
                 assert functionNode.getSymbol().isTemp();
                 // remove it
                 functionNode.setSymbol(null);
             }
-            final Symbol selfSymbol;
-            if (functionNode.isAnonymous()) {
-                selfSymbol = ensureSymbol(functionNode, Type.OBJECT, functionNode);
+            final boolean anonymous = functionNode.isAnonymous();
+            final String name = anonymous ? null : functionNode.getIdent().getName();
+            if (anonymous || body.getExistingSymbol(name) != null) {
+                // The function is either anonymous, or another local identifier already trumps its name on entry:
+                // either it has the same name as one of its parameters, or is named "arguments" and also references the
+                // "arguments" identifier in its body.
+                ensureSymbol(functionNode, Type.typeFor(ScriptFunction.class), functionNode);
             } else {
-                selfSymbol = defineSymbol(body, functionNode.getIdent().getName(), IS_VAR | IS_FUNCTION_SELF, functionNode);
+                final Symbol selfSymbol = defineSymbol(body, name, IS_VAR | IS_FUNCTION_SELF, functionNode);
+                assert selfSymbol.isFunctionSelf();
                 newType(selfSymbol, Type.OBJECT);
-                selfSymbolToFunction.put(selfSymbol, functionNode);
             }
         }
 

nashorn/src/jdk/nashorn/internal/ir/LexicalContext.java

@@ -27,7 +27,6 @@ package jdk.nashorn.internal.ir;
 import java.io.File;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
-
 import jdk.nashorn.internal.codegen.Label;
 import jdk.nashorn.internal.runtime.Debug;
 import jdk.nashorn.internal.runtime.Source;
@@ -171,6 +170,7 @@ public class LexicalContext {
     public <T extends LexicalContextNode> T pop(final T node) {
         --sp;
         final LexicalContextNode popped = stack[sp];
+        stack[sp] = null;
         if (popped instanceof Flags) {
             return (T)((Flags<?>)popped).setFlag(this, flags[sp]);
         }

nashorn/src/jdk/nashorn/internal/objects/NativeString.java

@@ -25,11 +25,11 @@
 
 package jdk.nashorn.internal.objects;
 
+import static jdk.nashorn.internal.lookup.Lookup.MH;
 import static jdk.nashorn.internal.runtime.ECMAErrors.typeError;
 import static jdk.nashorn.internal.runtime.JSType.isRepresentableAsInt;
 import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED;
 import static jdk.nashorn.internal.runtime.arrays.ArrayIndex.getArrayIndexNoThrow;
-import static jdk.nashorn.internal.lookup.Lookup.MH;
 
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
@@ -41,6 +41,7 @@ import java.util.List;
 import jdk.internal.dynalink.CallSiteDescriptor;
 import jdk.internal.dynalink.linker.GuardedInvocation;
 import jdk.internal.dynalink.linker.LinkRequest;
+import jdk.nashorn.internal.lookup.MethodHandleFactory;
 import jdk.nashorn.internal.objects.annotations.Attribute;
 import jdk.nashorn.internal.objects.annotations.Constructor;
 import jdk.nashorn.internal.objects.annotations.Function;
@@ -55,7 +56,6 @@ import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptObject;
 import jdk.nashorn.internal.runtime.ScriptRuntime;
 import jdk.nashorn.internal.runtime.arrays.ArrayIndex;
-import jdk.nashorn.internal.lookup.MethodHandleFactory;
 import jdk.nashorn.internal.runtime.linker.NashornGuards;
 import jdk.nashorn.internal.runtime.linker.PrimitiveLookup;
 
@@ -841,7 +841,7 @@ public final class NativeString extends ScriptObject {
         final long lim = (limit == UNDEFINED) ? JSType.MAX_UINT : JSType.toUint32(limit);
 
         if (separator == UNDEFINED) {
-            return limit == 0 ? new NativeArray() : new NativeArray(new Object[]{str});
+            return lim == 0 ? new NativeArray() : new NativeArray(new Object[]{str});
         }
 
         if (separator instanceof NativeRegExp) {

nashorn/src/jdk/nashorn/internal/parser/Parser.java

@@ -2339,7 +2339,7 @@ loop:
                 functionNode = functionNode.setFlag(lc, FunctionNode.IS_DECLARED);
             }
             if (ARGUMENTS.symbolName().equals(name.getName())) {
-                functionNode = functionNode.setFlag(lc, FunctionNode.DEFINES_ARGUMENTS);
+                lc.setFlag(lc.getCurrentFunction(), FunctionNode.DEFINES_ARGUMENTS);
             }
         }
 

nashorn/test/script/basic/JDK-8013325.js

@@ -0,0 +1,44 @@
+/*
+ * 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-8013325: function named 'arguments' should still access arguments object within itself.
+ * Its parent should however see the function and not an arguments object.
+ *
+ * @test
+ * @run
+ */
+
+function x() {
+  // x doesn't see an arguments object as it has a nested function with that name
+  // so it'll invoke the function.
+  arguments("a", "b", "c");
+  
+  function arguments(x, y, z) {
+      // The function 'arguments' OTOH can't see itself; if it uses the 
+      // identifier 'arguments', it'll see its own arguments object.
+      print(arguments)
+      print(x + " " + y + " " + z)
+  }
+}
+x()

nashorn/test/script/basic/JDK-8013325.js.EXPECTED

@@ -0,0 +1,2 @@
+[object Arguments]
+a b c