list.sort builtin

Author: qunaibit

graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_sort.txt

@@ -13,3 +13,5 @@
 *graalpython.lib-python.3.test.test_sort.TestOptimizedCompares.test_unsafe_long_compare
 *graalpython.lib-python.3.test.test_sort.TestOptimizedCompares.test_unsafe_object_compare
 *graalpython.lib-python.3.test.test_sort.TestOptimizedCompares.test_unsafe_tuple_compare
+*graalpython.lib-python.3.test.test_sort.TestBugs.test_bug453523
+*graalpython.lib-python.3.test.test_sort.TestBugs.test_undetected_mutation

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/ListBuiltins.java

@@ -65,6 +65,7 @@
 import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ListGeneralizationNode;
 import com.oracle.graal.python.builtins.objects.function.PArguments;
 import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
+import com.oracle.graal.python.builtins.objects.function.PKeyword;
 import com.oracle.graal.python.builtins.objects.generator.PGenerator;
 import com.oracle.graal.python.builtins.objects.ints.PInt;
 import com.oracle.graal.python.builtins.objects.iterator.PDoubleSequenceIterator;
@@ -80,9 +81,11 @@
 import com.oracle.graal.python.nodes.ErrorMessages;
 import com.oracle.graal.python.nodes.PGuards;
 import com.oracle.graal.python.nodes.argument.ReadArgumentNode;
+import com.oracle.graal.python.nodes.attributes.GetAttributeNode;
 import com.oracle.graal.python.nodes.builtins.ListNodes;
 import com.oracle.graal.python.nodes.builtins.ListNodes.AppendNode;
 import com.oracle.graal.python.nodes.builtins.ListNodes.IndexNode;
+import com.oracle.graal.python.nodes.call.CallNode;
 import com.oracle.graal.python.nodes.call.special.LookupAndCallUnaryNode;
 import com.oracle.graal.python.nodes.control.GetIteratorExpressionNode.GetIteratorNode;
 import com.oracle.graal.python.nodes.control.GetNextNode;
@@ -91,6 +94,7 @@
 import com.oracle.graal.python.nodes.function.builtins.PythonBinaryBuiltinNode;
 import com.oracle.graal.python.nodes.function.builtins.PythonTernaryBuiltinNode;
 import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
+import com.oracle.graal.python.nodes.function.builtins.PythonVarargsBuiltinNode;
 import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
 import com.oracle.graal.python.nodes.truffle.PythonArithmeticTypes;
 import com.oracle.graal.python.runtime.PythonCore;
@@ -840,6 +844,65 @@ public static ListReverseNode create() {
         }
     }
 
+    // list.sort(key=, reverse=)
+    @Builtin(name = "sort", minNumOfPositionalArgs = 1, takesVarArgs = true, takesVarKeywordArgs = true, needsFrame = true)
+    @GenerateNodeFactory
+    public abstract static class ListSortNode extends PythonVarargsBuiltinNode {
+
+        protected static final String SORT = "_sort";
+        protected static final String KEY = "key";
+
+        protected static boolean isSortable(PList list, SequenceStorageNodes.LenNode lenNode) {
+            return lenNode.execute(list.getSequenceStorage()) > 1;
+        }
+
+        protected static boolean maySideEffect(PList list, PKeyword[] keywords) {
+            if (PGuards.isObjectStorage(list)) {
+                return true;
+            }
+            if (keywords.length > 0) {
+                if (keywords[0].getName().equals(KEY)) {
+                    return true;
+                }
+                if (keywords.length > 1 && keywords[1].getName().equals(KEY)) {
+                    return true;
+                }
+            }
+            return false;
+        }
+
+        @Specialization(guards = "!isSortable(list, lenNode)")
+        @SuppressWarnings("unused")
+        Object none(VirtualFrame frame, PList list, Object[] arguments, PKeyword[] keywords,
+                        @Cached SequenceStorageNodes.LenNode lenNode) {
+            return PNone.NONE;
+        }
+
+        @Specialization(guards = {"isSortable(list, lenNode)", "maySideEffect(list, keywords)"})
+        Object withKey(VirtualFrame frame, PList list, Object[] arguments, PKeyword[] keywords,
+                        @Cached("create(SORT)") GetAttributeNode sort,
+                        @Cached CallNode callSort,
+                        @SuppressWarnings("unused") @Cached SequenceStorageNodes.LenNode lenNode) {
+            list.getSequenceStorage().setLock();
+            try {
+                defaultSort(frame, list, arguments, keywords, sort, callSort, lenNode);
+            } finally {
+                list.getSequenceStorage().releaseLock();
+            }
+            return PNone.NONE;
+        }
+
+        @Specialization(guards = {"isSortable(list, lenNode)", "!maySideEffect(list, keywords)"})
+        Object defaultSort(VirtualFrame frame, PList list, Object[] arguments, PKeyword[] keywords,
+                        @Cached("create(SORT)") GetAttributeNode sort,
+                        @Cached CallNode callSort,
+                        @SuppressWarnings("unused") @Cached SequenceStorageNodes.LenNode lenNode) {
+            Object sortMethod = sort.executeObject(frame, list);
+            callSort.execute(sortMethod, arguments, keywords);
+            return PNone.NONE;
+        }
+    }
+
     @Builtin(name = __LEN__, minNumOfPositionalArgs = 1)
     @GenerateNodeFactory
     public abstract static class LenNode extends PythonUnaryBuiltinNode {

graalpython/lib-graalpython/list.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # The Universal Permissive License (UPL), Version 1.0
@@ -38,7 +38,7 @@
 # SOFTWARE.
 
 
-def sort(self, key=None, reverse=False):
+def _sort(self, key=None, reverse=False):
     if reverse:
         if key:
             lt = lambda a,b: key(a) > key(b)
@@ -56,4 +56,4 @@ def sort(self, key=None, reverse=False):
     return None
 
 
-list.sort = sort
+list._sort = _sort

graalpython/lib-python/3/collections/__init__.py

@@ -1121,6 +1121,7 @@ def extend(self, other):
             self.data.extend(other.data)
         else:
             self.data.extend(other)
+    def _sort(self, /, *args, **kwds): self.data._sort(*args, **kwds) # XXX to be removed once sorting is implemented internally (mq)