Implement the key= keyword argument for sort() and sorted()

I don't think the current implementation will be that fast
and will probably do poorly in sort()/sorted() microbenchmarks.
My feeling is that it doesn't matter that much, but if it does
there are some things we can do to optimize it.

src/runtime/builtin_modules/builtins.cpp

@@ -413,27 +413,7 @@ Box* sorted(Box* obj, Box* key, Box* cmp, Box** args) {
         listAppendInternal(rtn, e);
     }
 
-    std::sort<Box**, PyLt>(rtn->elts->elts, rtn->elts->elts + rtn->size, PyLt());
-
-    return rtn;
-}
-
-Box* sortedList(Box* obj) {
-    RELEASE_ASSERT(obj->cls == list_cls, "");
-
-    BoxedList* lobj = static_cast<BoxedList*>(obj);
-    BoxedList* rtn = new BoxedList();
-
-    int size = lobj->size;
-    rtn->elts = new (size) GCdArray();
-    rtn->size = size;
-    rtn->capacity = size;
-    for (int i = 0; i < size; i++) {
-        Box* t = rtn->elts->elts[i] = lobj->elts->elts[i];
-    }
-
-    std::sort<Box**, PyLt>(rtn->elts->elts, rtn->elts->elts + size, PyLt());
-
+    listSort(rtn, key, cmp, reverse);
     return rtn;
 }
 
@@ -1113,7 +1093,6 @@ void setupBuiltins() {
 
 
     CLFunction* sorted_func = createRTFunction(4, 3, false, false, ParamNames({ "", "cmp", "key", "reverse" }, "", ""));
-    addRTFunction(sorted_func, (void*)sortedList, LIST, { LIST, UNKNOWN, UNKNOWN, UNKNOWN });
     addRTFunction(sorted_func, (void*)sorted, LIST, { UNKNOWN, UNKNOWN, UNKNOWN, UNKNOWN });
     builtins_module->giveAttr("sorted", new BoxedBuiltinFunctionOrMethod(sorted_func, { None, None, False }));
 

src/runtime/list.cpp

@@ -433,13 +433,74 @@ Box* listAdd(BoxedList* self, Box* _rhs) {
     return rtn;
 }
 
-Box* listSort1(BoxedList* self) {
+Box* listReverse(BoxedList* self) {
     LOCK_REGION(self->lock.asWrite());
 
     assert(self->cls == list_cls);
+    for (int i = 0, j = self->size - 1; i < j; i++, j--) {
+        Box* e = self->elts->elts[i];
+        self->elts->elts[i] = self->elts->elts[j];
+        self->elts->elts[j] = e;
+    }
+
+    return None;
+}
+
+void listSort(BoxedList* self, Box* cmp, Box* key, Box* reverse) {
+    LOCK_REGION(self->lock.asWrite());
+    assert(self->cls == list_cls);
+
+    RELEASE_ASSERT(cmp == None, "The 'cmp' keyword is currently not supported");
+
+    if (key == None)
+        key = NULL;
+
+    int num_keys_added = 0;
+    auto remove_keys = [&]() {
+        for (int i = 0; i < num_keys_added; i++) {
+            Box** obj_loc = &self->elts->elts[i];
+            assert((*obj_loc)->cls == tuple_cls);
+            *obj_loc = static_cast<BoxedTuple*>(*obj_loc)->elts[2];
+        }
+    };
 
-    std::sort<Box**, PyLt>(self->elts->elts, self->elts->elts + self->size, PyLt());
+    try {
+        if (key) {
+            for (int i = 0; i < self->size; i++) {
+                Box** obj_loc = &self->elts->elts[i];
+
+                Box* key_val = runtimeCall(key, ArgPassSpec(1), *obj_loc, NULL, NULL, NULL, NULL);
+                // Add the index as part of the new tuple so that the comparison never hits the
+                // original object.
+                // TODO we could potentially make this faster by copying the CPython approach of
+                // creating special sortwrapper objects that compare only based on the key.
+                Box* new_obj = new BoxedTuple({ key_val, boxInt(i), *obj_loc });
+
+                *obj_loc = new_obj;
+                num_keys_added++;
+            }
+        }
 
+        // We don't need to do a stable sort if there's a keyfunc, since we explicitly added the index
+        // as part of the sort key.
+        // But we might want to get rid of that approach?  CPython doesn't do that (they create special
+        // wrapper objects that compare only based on the key).
+        std::stable_sort<Box**, PyLt>(self->elts->elts, self->elts->elts + self->size, PyLt());
+    } catch (ExcInfo e) {
+        remove_keys();
+        throw e;
+    }
+
+    remove_keys();
+
+    if (nonzero(reverse)) {
+        listReverse(self);
+    }
+}
+
+Box* listSortFunc(BoxedList* self, Box* cmp, Box* key, Box** _args) {
+    Box* reverse = _args[0];
+    listSort(self, cmp, key, reverse);
     return None;
 }
 
@@ -450,7 +511,7 @@ extern "C" int PyList_Sort(PyObject* v) noexcept {
     }
 
     try {
-        listSort1((BoxedList*)v);
+        listSort((BoxedList*)v, None, None, False);
     } catch (ExcInfo e) {
         setCAPIException(e);
         return -1;
@@ -531,19 +592,6 @@ Box* listRemove(BoxedList* self, Box* elt) {
     raiseExcHelper(ValueError, "list.remove(x): x not in list");
 }
 
-Box* listReverse(BoxedList* self) {
-    LOCK_REGION(self->lock.asWrite());
-
-    assert(self->cls == list_cls);
-    for (int i = 0, j = self->size - 1; i < j; i++, j--) {
-        Box* e = self->elts->elts[i];
-        self->elts->elts[i] = self->elts->elts[j];
-        self->elts->elts[j] = e;
-    }
-
-    return None;
-}
-
 BoxedClass* list_iterator_cls = NULL;
 extern "C" void listIteratorGCHandler(GCVisitor* v, Box* b) {
     boxGCHandler(v, b);
@@ -735,7 +783,9 @@ void setupList() {
     list_cls->giveAttr("__iadd__", new BoxedFunction(boxRTFunction((void*)listIAdd, UNKNOWN, 2)));
     list_cls->giveAttr("__add__", new BoxedFunction(boxRTFunction((void*)listAdd, UNKNOWN, 2)));
 
-    list_cls->giveAttr("sort", new BoxedFunction(boxRTFunction((void*)listSort1, NONE, 1)));
+    list_cls->giveAttr("sort", new BoxedFunction(boxRTFunction((void*)listSortFunc, NONE, 4, 3, false, false,
+                                                               ParamNames({ "", "cmp", "key", "reverse" }, "", "")),
+                                                 { None, None, False }));
     list_cls->giveAttr("__contains__", new BoxedFunction(boxRTFunction((void*)listContains, BOXED_BOOL, 2)));
 
     list_cls->giveAttr("__new__",

src/runtime/list.h

@@ -35,6 +35,7 @@ Box* listIterIter(Box* self);
 Box* listiterHasnext(Box* self);
 i1 listiterHasnextUnboxed(Box* self);
 Box* listiterNext(Box* self);
+void listSort(BoxedList* self, Box* cmp, Box* key, Box* reverse);
 extern "C" Box* listAppend(Box* self, Box* v);
 }
 

test/tests/builtins.py

@@ -84,3 +84,7 @@ class C(object):
     s = sorted
 c = C()
 print c.s([3,2,1])
+
+l = range(5)
+print sorted(l, key=lambda x:-x)
+print l

test/tests/list.py

@@ -113,3 +113,19 @@ for i in xrange(3):
             l = range(i)
             l[j:k] = ["added"]
             print i, j, k, l
+
+
+
+l = [1, 3, 5, 7, 2, 4]
+print l.sort(key=lambda x:x%3)
+print l
+print l.sort(reverse=True)
+print l
+
+# If the keyfunc throws an exception, we shouldn't see any modifications:
+l = range(9)
+try:
+    print sorted(l, key=lambda i:1.0/(5-i))
+except ZeroDivisionError:
+    pass
+print l

test/tests/sort_cmp.py

+# expected: fail
+# - we don't support the cmp= keyword yet
+# (apparently it's removed in Python 3)
+
+printed = False
+def mycmp(x, y):
+    global printed
+    if not printed:
+        print type(x), type(y)
+        printed = True
+
+    if x < y:
+        return -1
+    if x > y:
+        return 1
+    return 0
+
+l = range(9)
+print l.sort(cmp=mycmp, key=lambda x:x%2, reverse=True)
+print l