fix list.sort and sorted to dispatch user __lt__

Author: dylan-sutton-chavez

bugs.json

@@ -2,8 +2,8 @@
   "_meta": {
     "source": "MicroPython tests/basics differential vs CPython 3.13",
     "verified": "each repro re-run through VM + python3",
-    "count": 7,
-    "fixed": "93 bugs fixed and migrated to vm.json (try/finally + with unwinding cluster; 11 builtin/parser/format divergences from rows 1-11 of the low-NEWLINE triage; generator_return, string_format_modulo_int %*d, seq_unpack list targets, class_getattr dunder-on-type, builtin_slice type identity, containment dict/set-led tuple, range sequence-unpack; yield-from-as-expression cluster: gen_yield_from + gen_yield_from_ducktype + gen_yield_from_exc + gen_yield_from_stopped via a parse_atom Yield arm plus a LoadYieldFrom opcode that carries the subiterator return/StopIteration value, 40 regression cases; range_iter_overflow: AFL sig:06 abort on a range whose step increment crossed the i64 edge, fixed by checked_add in the three range walkers (iter_to_vec_for_spread, IterFrame::next_item, IterCursor::Range::next), 10 regression cases; range_iter_truncation: for-loop/comprehension (IterFrame::next_item) and spread/unpack (iter_to_vec_for_spread) yielded Val::int past the 47-bit inline range instead of promoting, routed both through range_int (pub(crate)) so values auto-promote to LongInt, 10 regression cases; range_contains: `x in range(...)` always returned False (contains had a catch-all Ok(false) for Range) — replaced with an O(1) bounds+step check over i128 that also matches integral floats like CPython, 10 regression cases; set_binop: augmented set `|=`/`&=`/`^=` rebound to a new set instead of mutating in place — added InPlaceBitOr/And/Xor opcodes (parser maps the augmented ops, handle_bitwise routes to set_iop_and_push) that rewrite the left set in place, frozenset still rebinds, 10 regression cases; del_deref: `del` of a closed-over variable cleared only the local slot, not the shared cell, so closures still read the old value — the Del handler now also unbinds the frame's cell for that slot, 10 regression cases)"
+    "count": 6,
+    "fixed": "94 bugs fixed and migrated to vm.json (try/finally + with unwinding cluster; 11 builtin/parser/format divergences from rows 1-11 of the low-NEWLINE triage; generator_return, string_format_modulo_int %*d, seq_unpack list targets, class_getattr dunder-on-type, builtin_slice type identity, containment dict/set-led tuple, range sequence-unpack; yield-from-as-expression cluster: gen_yield_from + gen_yield_from_ducktype + gen_yield_from_exc + gen_yield_from_stopped via a parse_atom Yield arm plus a LoadYieldFrom opcode that carries the subiterator return/StopIteration value, 40 regression cases; range_iter_overflow: AFL sig:06 abort on a range whose step increment crossed the i64 edge, fixed by checked_add in the three range walkers (iter_to_vec_for_spread, IterFrame::next_item, IterCursor::Range::next), 10 regression cases; range_iter_truncation: for-loop/comprehension (IterFrame::next_item) and spread/unpack (iter_to_vec_for_spread) yielded Val::int past the 47-bit inline range instead of promoting, routed both through range_int (pub(crate)) so values auto-promote to LongInt, 10 regression cases; range_contains: `x in range(...)` always returned False (contains had a catch-all Ok(false) for Range) — replaced with an O(1) bounds+step check over i128 that also matches integral floats like CPython, 10 regression cases; set_binop: augmented set `|=`/`&=`/`^=` rebound to a new set instead of mutating in place — added InPlaceBitOr/And/Xor opcodes (parser maps the augmented ops, handle_bitwise routes to set_iop_and_push) that rewrite the left set in place, frozenset still rebinds, 10 regression cases; del_deref: `del` of a closed-over variable cleared only the local slot, not the shared cell, so closures still read the old value — the Del handler now also unbinds the frame's cell for that slot, 10 regression cases; list_sort: `list.sort()` and `sorted()` used a pure `lt_vals` comparator that never dispatched a user `__lt__`, raising TypeError on class instances — added `sort_lt` (tries the `__lt__` dunder, then `lt_vals`) used by both sort paths, threaded chunk/slots through `call_sorted`/`sort_by_lt`/`sort_by_key` with GC rooting, and routed all `list.sort()` through `exec_sort` (both call paths) since the builtin-method table can't carry a frame, 10 regression cases)"
   },
   "bugs": [
     {
@@ -46,14 +46,6 @@
       "expected": "(1, 2) {'z': 3}",
       "actual": "__VMERR__ TypeError: object is not callable"
     },
-    {
-      "id": "list_sort",
-      "category": "wrong-value",
-      "summary": "list.sort() does not dispatch to a user-defined __lt__ (which works for direct < comparison), raising TypeError instead of sorting.",
-      "repro": "class A:\n    def __init__(self, x):\n        self.x = x\n    def __lt__(self, other):\n        return self.x < other.x\n    def __repr__(self):\n        return str(self.x)\nl = [A(3), A(1), A(2)]\nl.sort()\nprint(l)",
-      "expected": "[1, 2, 3]",
-      "actual": "__VMERR__ TypeError: '<' not supported between instances of 'object' and 'object'"
-    },
     {
       "id": "gen_next_nested_arg",
       "category": "stack-corruption",

compiler/src/modules/vm/builtins/sequence.rs

@@ -4,6 +4,7 @@ use alloc::{vec, vec::Vec};
 
 use super::super::VM;
 use super::super::types::*;
+use crate::modules::parser::{OpCode, SSAChunk};
 
 /* A range element as a Val, promoting magnitudes beyond the 47-bit inline range to LongInt. */
 pub(crate) fn range_int(heap: &mut HeapPool, i: i64) -> Result<Val, VmErr> {
@@ -86,10 +87,10 @@ impl<'a> VM<'a> {
         self.push(Val::int(n)); Ok(())
     }
 
-    pub fn call_sorted(&mut self, reverse: bool) -> Result<(), VmErr> {
+    pub fn call_sorted(&mut self, reverse: bool, chunk: &SSAChunk, slots: &mut [Val]) -> Result<(), VmErr> {
         let o = self.pop()?;
         let mut items = self.extract_iter(o, false)?;
-        self.sort_by_lt(&mut items)?;
+        self.sort_by_lt(&mut items, chunk, slots)?;
         if reverse { items.reverse(); }
         self.alloc_and_push_list(items)
     }
@@ -98,7 +99,7 @@ impl<'a> VM<'a> {
     pub fn call_sorted_with_key(&mut self, key: Option<Val>, reverse: bool, chunk: &crate::modules::parser::SSAChunk, slots: &mut [Val]) -> Result<(), VmErr> {
         let key = match key {
             Some(k) if !k.is_none() => k,
-            _ => return self.call_sorted(reverse),
+            _ => return self.call_sorted(reverse, chunk, slots),
         };
         let o = self.pop()?;
         let items = self.extract_iter(o, false)?;
@@ -117,7 +118,7 @@ impl<'a> VM<'a> {
             self.sort_by_key(items, k, chunk, slots)?
         } else {
             let mut s = items;
-            self.sort_by_lt(&mut s)?;
+            self.sort_by_lt(&mut s, chunk, slots)?;
             s
         };
         if reverse { result.reverse(); }
@@ -140,39 +141,53 @@ impl<'a> VM<'a> {
             self.exec_call(1, chunk, slots)?;
             keys.push(self.pop()?);
         }
+        let roots_base = self.temp_roots.len();
+        for &v in keys.iter().chain(items.iter()) { self.temp_roots.push(v); }
         let mut sort_err: Option<VmErr> = None;
         let order = Self::stable_sort_indices(items.len(), |a, b| {
             if sort_err.is_some() { return core::cmp::Ordering::Equal; }
-            match self.lt_vals(keys[a], keys[b]) {
+            match self.sort_lt(keys[a], keys[b], chunk, slots) {
                 Ok(true) => core::cmp::Ordering::Less,
-                Ok(false) => match self.lt_vals(keys[b], keys[a]) {
+                Ok(false) => match self.sort_lt(keys[b], keys[a], chunk, slots) {
                     Ok(true) => core::cmp::Ordering::Greater,
                     Ok(false) => core::cmp::Ordering::Equal,
                     Err(e) => { sort_err = Some(e); core::cmp::Ordering::Equal }
                 },
                 Err(e) => { sort_err = Some(e); core::cmp::Ordering::Equal }
             }
         });
+        self.temp_roots.truncate(roots_base);
         if let Some(e) = sort_err { return Err(e); }
         Ok(order.into_iter().map(|i| items[i]).collect())
     }
 
-    /* In-place sort via `lt_vals`. Stashes the first error and surfaces it after the sort. */
-    pub(crate) fn sort_by_lt(&self, items: &mut [Val]) -> Result<(), VmErr> {
+    // a < b via __lt__ when either side defines it, else the built-in comparison.
+    pub(crate) fn sort_lt(&mut self, a: Val, b: Val, chunk: &SSAChunk, slots: &mut [Val]) -> Result<bool, VmErr> {
+        if let Some(r) = self.try_compare_dunder(OpCode::Lt, a, b, chunk, slots)? {
+            return Ok(self.truthy(r));
+        }
+        self.lt_vals(a, b)
+    }
+
+    /* In-place sort dispatching `__lt__`; roots items since a comparison can run user code that GCs. */
+    pub(crate) fn sort_by_lt(&mut self, items: &mut [Val], chunk: &SSAChunk, slots: &mut [Val]) -> Result<(), VmErr> {
         let snapshot = items.to_vec();
+        let roots_base = self.temp_roots.len();
+        for &v in &snapshot { self.temp_roots.push(v); }
         let mut sort_err: Option<VmErr> = None;
-        let order = Self::stable_sort_indices(items.len(), |a, b| {
+        let order = Self::stable_sort_indices(snapshot.len(), |a, b| {
             if sort_err.is_some() { return core::cmp::Ordering::Equal; }
-            match self.lt_vals(snapshot[a], snapshot[b]) {
+            match self.sort_lt(snapshot[a], snapshot[b], chunk, slots) {
                 Ok(true) => core::cmp::Ordering::Less,
-                Ok(false) => match self.lt_vals(snapshot[b], snapshot[a]) {
+                Ok(false) => match self.sort_lt(snapshot[b], snapshot[a], chunk, slots) {
                     Ok(true) => core::cmp::Ordering::Greater,
                     Ok(false) => core::cmp::Ordering::Equal,
                     Err(e) => { sort_err = Some(e); core::cmp::Ordering::Equal }
                 },
                 Err(e) => { sort_err = Some(e); core::cmp::Ordering::Equal }
             }
         });
+        self.temp_roots.truncate(roots_base);
         if let Some(e) = sort_err { return Err(e); }
         for (dst, &src) in order.iter().enumerate() { items[dst] = snapshot[src]; }
         Ok(())

compiler/src/modules/vm/dispatch.rs

@@ -315,6 +315,8 @@ impl<'a> VM<'a> {
                 self.exec_call(encoded, chunk, slots)
             }
             handlers::methods::AttrLookup::BuiltinMethod(id) => {
+                // sort runs user __lt__, so it needs chunk/slots the builtin-method table can't carry.
+                if id.name() == "sort" { return self.exec_sort(obj, &positional, &kw_flat, chunk, slots); }
                 self.exec_bound_method(obj, id, &positional, &kw_flat)
             }
             handlers::methods::AttrLookup::InstanceField(field) => {

compiler/src/modules/vm/handlers/builtin_methods/list.rs

@@ -116,11 +116,7 @@ pub fn pop(vm: &mut VM, recv: Val, pos: &[Val]) -> Result<(), VmErr> {
     vm.push(popped); Ok(())
 }
 
-pub fn sort(vm: &mut VM, recv: Val, _pos: &[Val]) -> Result<(), VmErr> {
-    let mut sorted = list_clone(vm, recv)?;
-    vm.sort_by_lt(&mut sorted)?;
-    list_mut(vm, recv, "sort: receiver is not a list", |list| {
-        *list = sorted; Ok(())
-    })?;
-    vm.push(Val::none()); Ok(())
+// list.sort() is intercepted in try_dispatch_non_func_callable, which has chunk/slots for __lt__.
+pub fn sort(_vm: &mut VM, _recv: Val, _pos: &[Val]) -> Result<(), VmErr> {
+    Err(cold_type("list.sort dispatched without a frame"))
 }

compiler/src/modules/vm/handlers/function.rs

@@ -67,7 +67,7 @@ impl<'a> VM<'a> {
             OpCode::CallType => self.call_type(),
             OpCode::CallChr => self.call_chr(),
             OpCode::CallOrd => self.call_ord(),
-            OpCode::CallSorted => self.call_sorted(false),
+            OpCode::CallSorted => self.call_sorted(false, chunk, slots),
             OpCode::CallList => self.call_list(operand, chunk, slots),
             OpCode::CallTuple => self.call_tuple(operand, chunk, slots),
             OpCode::CallEnumerate => self.call_enumerate(operand),
@@ -304,26 +304,30 @@ impl<'a> VM<'a> {
         Ok(Some(heap.alloc(super::super::types::HeapObj::Dict(Rc::new(RefCell::new(dm))))?))
     }
 
+    /* list.sort(): parse key/reverse kwargs and sort in place. Intercepted from both call paths since it needs chunk/slots for __lt__. */
+    pub(crate) fn exec_sort(&mut self, recv: Val, positional: &[Val], kw_flat: &[Val], chunk: &SSAChunk, slots: &mut [Val]) -> Result<(), VmErr> {
+        if !positional.is_empty() {
+            return Err(cold_type("list.sort() takes no positional arguments"));
+        }
+        let mut sort_key: Option<Val> = None;
+        let mut sort_reverse = false;
+        for pair in kw_flat.chunks(2) {
+            match self.kw_name(pair[0]) {
+                Some("key") => sort_key = Some(pair[1]),
+                Some("reverse") => sort_reverse = self.truthy(pair[1]),
+                _ => return Err(cold_type("list.sort() got unexpected keyword argument")),
+            }
+        }
+        self.call_list_sort_keyed(recv, sort_key, sort_reverse, chunk, slots)
+    }
+
     /* Dispatch non-Func callees. Returns Ok(true) when handled here; Ok(false) means the caller falls through to the Func path. */
     fn try_dispatch_non_func_callable(&mut self, callee: Val, positional: &[Val], kw_flat: &[Val], num_kw: usize, chunk: &SSAChunk, slots: &mut [Val]) -> Result<bool, VmErr> {
         if let HeapObj::BoundMethod(recv, id) = self.heap.get(callee) {
             let recv = *recv;
             let id = *id;
-            if id.name() == "sort" && !kw_flat.is_empty() {
-                if !positional.is_empty() {
-                    return Err(cold_type("list.sort() takes no positional arguments"));
-                }
-                let mut sort_key: Option<Val> = None;
-                let mut sort_reverse = false;
-                for pair in kw_flat.chunks(2) {
-                    let (name_v, val_v) = (pair[0], pair[1]);
-                    match self.kw_name(name_v) {
-                        Some("key") => sort_key = Some(val_v),
-                        Some("reverse") => sort_reverse = self.truthy(val_v),
-                        _ => return Err(cold_type("list.sort() got unexpected keyword argument")),
-                    }
-                }
-                self.call_list_sort_keyed(recv, sort_key, sort_reverse, chunk, slots)?;
+            if id.name() == "sort" {
+                self.exec_sort(recv, positional, kw_flat, chunk, slots)?;
                 return Ok(true);
             }
             self.exec_bound_method(recv, id, positional, kw_flat)?;

compiler/tests/cases/vm.json

@@ -3587,5 +3587,15 @@
   {"src":"def f():\n    x = 1\n    y = 2\n    def g():\n        nonlocal x, y\n        try:\n            print(x)\n        except NameError:\n            print('x gone')\n        print(y)\n    del x\n    g()\nf()","output":["x gone","2"]},
   {"src":"def f():\n    x = 1\n    def g():\n        nonlocal x\n        try:\n            return x\n        except NameError:\n            return 'NE'\n    def h():\n        nonlocal x\n        try:\n            return x\n        except NameError:\n            return 'NE'\n    del x\n    print(g(), h())\nf()","output":["NE NE"]},
   {"src":"def f():\n    x = 1\n    def g():\n        nonlocal x\n        return x\n    del x\n    try:\n        g()\n    except NameError:\n        return 'caught'\nprint(f())","output":["caught"]},
-  {"src":"def f():\n    x = 1\n    def g():\n        return x\n    del x\n    x = 99\n    return g()\nprint(f())","output":["99"]}
+  {"src":"def f():\n    x = 1\n    def g():\n        return x\n    del x\n    x = 99\n    return g()\nprint(f())","output":["99"]},
+  {"src":"class A:\n    def __init__(self, x):\n        self.x = x\n    def __lt__(self, other):\n        return self.x < other.x\n    def __repr__(self):\n        return str(self.x)\nl = [A(3), A(1), A(2)]\nl.sort()\nprint(l)","output":["[1, 2, 3]"]},
+  {"src":"class A:\n    def __init__(self, x):\n        self.x = x\n    def __lt__(self, other):\n        return self.x < other.x\n    def __repr__(self):\n        return str(self.x)\nprint(sorted([A(3), A(1), A(2)]))","output":["[1, 2, 3]"]},
+  {"src":"class A:\n    def __init__(self, x):\n        self.x = x\n    def __lt__(self, other):\n        return self.x < other.x\n    def __repr__(self):\n        return str(self.x)\nl = [A(3), A(1), A(2)]\nl.sort(reverse=True)\nprint(l)","output":["[3, 2, 1]"]},
+  {"src":"class A:\n    def __init__(self, x):\n        self.x = x\n    def __lt__(self, other):\n        return self.x < other.x\n    def __repr__(self):\n        return str(self.x)\nprint(sorted([A(3), A(1), A(2)], reverse=True))","output":["[3, 2, 1]"]},
+  {"src":"l = [3, 1, 2]\nl.sort()\nprint(l)","output":["[1, 2, 3]"]},
+  {"src":"print(sorted(['banana', 'apple', 'cherry']))","output":["['apple', 'banana', 'cherry']"]},
+  {"src":"print(sorted([-3, 1, -2], key=abs))","output":["[1, -2, -3]"]},
+  {"src":"print(sorted(['ccc', 'a', 'bb'], key=len))","output":["['a', 'bb', 'ccc']"]},
+  {"src":"try:\n    sorted([1, 'a'])\nexcept TypeError:\n    print('te')","output":["te"]},
+  {"src":"l = [2, 1]\nr = l.sort()\nprint(r, l)","output":["None [1, 2]"]}
 ]

docs/content/reference/builtins.md

@@ -316,7 +316,7 @@ True
 
 ### sorted
 
-New sorted list. Accepts `key=fn` and `reverse=True/False`. Numbers, strings, bytes, and tuples/lists order lexicographically; mixed un-orderable types raise `TypeError`.
+New sorted list. Accepts `key=fn` and `reverse=True/False`. Numbers, strings, bytes, and tuples/lists order lexicographically; objects defining `__lt__` sort by it; mixed un-orderable types raise `TypeError`.
 
 ```python
 print(sorted([3, 1, 4, 1, 5]))

docs/content/reference/methods.md

@@ -268,7 +268,7 @@ print(ys)
 
 ### Mutating
 
-Return `None`, mutate in place. `extend` accepts any iterable. `sort` accepts `key=fn` and `reverse=True/False`.
+Return `None`, mutate in place. `extend` accepts any iterable. `sort` accepts `key=fn` and `reverse=True/False`, and orders custom objects by their `__lt__`.
 
 ```python
 xs = [1, 2, 3]