8171849: Collection and Queue conversions not prioritized for Arrays

Reviewed-by: hannesw, jlaskey

3 files changed, 128 insertions, 7 deletions

diff --git a/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java b/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java
index ddcbae1f..b8e01ffd 100644
--- a/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java
+++ b/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java
@@ -251,15 +251,15 @@ final class NashornLinker implements TypeBasedGuardingDynamicLinker, GuardingTyp
     @Override
     public Comparison compareConversion(final Class<?> sourceType, final Class<?> targetType1, final Class<?> targetType2) {
         if(sourceType == NativeArray.class) {
-            // Prefer lists, as they're less costly to create than arrays.
-            if(isList(targetType1)) {
-                if(!isList(targetType2)) {
+            // Prefer those types we can convert to with just a wrapper (cheaper than Java array creation).
+            if(isArrayPreferredTarget(targetType1)) {
+                if(!isArrayPreferredTarget(targetType2)) {
                     return Comparison.TYPE_1_BETTER;
                 }
-            } else if(isList(targetType2)) {
+            } else if(isArrayPreferredTarget(targetType2)) {
                 return Comparison.TYPE_2_BETTER;
             }
-            // Then prefer arrays
+            // Then prefer Java arrays
             if(targetType1.isArray()) {
                 if(!targetType2.isArray()) {
                     return Comparison.TYPE_1_BETTER;
@@ -281,8 +281,8 @@ final class NashornLinker implements TypeBasedGuardingDynamicLinker, GuardingTyp
         return Comparison.INDETERMINATE;
     }
 
-    private static boolean isList(final Class<?> clazz) {
-        return clazz == List.class || clazz == Deque.class;
+    private static boolean isArrayPreferredTarget(final Class<?> clazz) {
+        return clazz == List.class || clazz == Collection.class || clazz == Queue.class || clazz == Deque.class;
     }
 
     private static final MethodHandle IS_SCRIPT_OBJECT = Guards.isInstance(ScriptObject.class, MH.type(Boolean.TYPE, Object.class));
diff --git a/test/script/basic/JDK-8171849.js b/test/script/basic/JDK-8171849.js
new file mode 100644
index 00000000..febc97ce
--- /dev/null
+++ b/test/script/basic/JDK-8171849.js
@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2016 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-8171849: Collection and Queue conversions not prioritized for Arrays
+ *
+ * @test
+ * @run
+ */
+
+var acp = new (Java.type("jdk.nashorn.test.models.ArrayConversionPreferences"))
+
+var a = [1, "", {}]
+
+Assert.assertTrue(acp.testCollectionOverMap(a))
+Assert.assertTrue(acp.testCollectionOverArray(a))
+Assert.assertTrue(acp.testListOverMap(a))
+Assert.assertTrue(acp.testListOverArray(a))
+Assert.assertTrue(acp.testListOverCollection(a))
+Assert.assertTrue(acp.testQueueOverMap(a))
+Assert.assertTrue(acp.testQueueOverArray(a))
+Assert.assertTrue(acp.testQueueOverCollection(a))
+Assert.assertTrue(acp.testDequeOverMap(a))
+Assert.assertTrue(acp.testDequeOverArray(a))
+Assert.assertTrue(acp.testDequeOverCollection(a))
+Assert.assertTrue(acp.testDequeOverQueue(a))
+Assert.assertTrue(acp.testArrayOverMap(a))
diff --git a/test/src/jdk/nashorn/test/models/ArrayConversionPreferences.java b/test/src/jdk/nashorn/test/models/ArrayConversionPreferences.java
new file mode 100644
index 00000000..a88f3c37
--- /dev/null
+++ b/test/src/jdk/nashorn/test/models/ArrayConversionPreferences.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2016, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+package jdk.nashorn.test.models;
+
+import java.util.Collection;
+import java.util.Deque;
+import java.util.List;
+import java.util.Map;
+import java.util.Queue;
+
+public class ArrayConversionPreferences {
+    public boolean testCollectionOverMap(final Collection x) { return true; }
+    public boolean testCollectionOverMap(final Map x) { return false; }
+
+    public boolean testCollectionOverArray(final Collection x) { return true; }
+    public boolean testCollectionOverArray(final Object[] x) { return false; }
+
+    public boolean testListOverMap(final List x) { return true; }
+    public boolean testListOverMap(final Map x) { return false; }
+
+    public boolean testListOverArray(final List x) { return true; }
+    public boolean testListOverArray(final Object[] x) { return false; }
+
+    public boolean testListOverCollection(final List x) { return true; }
+    public boolean testListOverCollection(final Collection x) { return false; }
+
+    public boolean testQueueOverMap(final Queue x) { return true; }
+    public boolean testQueueOverMap(final Map x) { return false; }
+
+    public boolean testQueueOverArray(final Queue x) { return true; }
+    public boolean testQueueOverArray(final Object[] x) { return false; }
+
+    public boolean testQueueOverCollection(final Queue x) { return true; }
+    public boolean testQueueOverCollection(final Collection x) { return false; }
+
+    public boolean testDequeOverMap(final Deque x) { return true; }
+    public boolean testDequeOverMap(final Map x) { return false; }
+
+    public boolean testDequeOverArray(final Deque x) { return true; }
+    public boolean testDequeOverArray(final Object[] x) { return false; }
+
+    public boolean testDequeOverCollection(final Deque x) { return true; }
+    public boolean testDequeOverCollection(final Collection x) { return false; }
+
+    public boolean testDequeOverQueue(final Deque x) { return true; }
+    public boolean testDequeOverQueue(final Queue x) { return false; }
+
+    public boolean testArrayOverMap(final Object[] x) { return true; }
+    public boolean testArrayOverMap(final Map x) { return false; }
+}
+