Skip to content

Commit bb75624

Browse files
name-of-okjaname-of-okja
authored
Implement cell comparison and repr (RustPython#8458)
The cell type filled neither the richcompare nor the repr slot, so object's address-based defaults showed through: cell(1) == cell(1) was False, ordering raised TypeError, and repr rendered <cell object at 0x...> rather than <cell at 0x...: int object at 0x...>. Compare cells by contents, with empty cells ordering before everything else, and render CPython's repr for both the filled and empty cases. The comparison fills the richcompare slot directly rather than going through Comparable, whose cmp() can only answer with a bool. CPython returns whatever PyObject_RichCompare produced, so a contained __eq__ that yields a non-bool must pass through untouched; coercing it would also call __bool__ and surface exceptions CPython never raises. The empty-cell branch still answers with a bool, so both arms are needed and the slot returns Either. The repr truncates the contained type name the way "%.80s" does: at most 80 bytes, dropping a character the cut would leave incomplete. Mark the type unhashable. Content-based equality combined with the inherited identity hash would break the hash/eq contract, and hashing the contents is not possible either because cell_contents is writable. CPython gets this implicitly, because defining tp_richcompare suppresses tp_hash inheritance. Reference: CPython Objects/cellobject.c, cell_richcompare and cell_repr. Assisted-by: Claude Code:claude-opus-5
1 parent 9bab06c commit bb75624

3 files changed

Lines changed: 45 additions & 5 deletions

File tree

‎Lib/test/test_funcattrs.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,6 @@ def f():
432432

433433

434434
class CellTest(unittest.TestCase):
435-
@unittest.expectedFailure # TODO: RUSTPYTHON
436435
def test_comparison(self):
437436
# These tests are here simply to exercise the comparison code;
438437
# their presence should not be interpreted as providing any

‎Lib/test/test_reprlib.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,6 @@ def test_nesting(self):
237237
eq(r([[[[[[{}]]]]]]), "[[[[[[{}]]]]]]")
238238
eq(r([[[[[[[{}]]]]]]]), "[[[[[[[...]]]]]]]")
239239

240-
@unittest.expectedFailure # TODO: RUSTPYTHON
241240
def test_cell(self):
242241
def get_cell():
243242
x = 42

‎crates/vm/src/builtins/function.rs‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use crate::{
1515
class::PyClassImpl,
1616
common::wtf8::{Wtf8Buf, wtf8_concat},
1717
frame::{FrameObject, FrameObjectRef},
18-
function::{FuncArgs, OptionalArg, PyComparisonValue, PySetterValue},
18+
function::{Either, FuncArgs, OptionalArg, PyComparisonValue, PySetterValue},
1919
scope::Scope,
2020
types::{
2121
Callable, Comparable, Constructor, GetAttr, GetDescriptor, Hashable, PyComparisonOp,
@@ -1507,7 +1507,7 @@ impl Representable for PyBoundMethod {
15071507
}
15081508
}
15091509

1510-
#[pyclass(module = false, name = "cell", traverse)]
1510+
#[pyclass(module = false, name = "cell", unhashable = true, traverse)]
15111511
#[derive(Debug, Default)]
15121512
pub(crate) struct PyCell {
15131513
contents: PyMutex<Option<PyObjectRef>>,
@@ -1530,8 +1530,26 @@ impl Constructor for PyCell {
15301530
}
15311531
}
15321532

1533-
#[pyclass(with(Constructor))]
1533+
#[pyclass(with(Constructor, Representable))]
15341534
impl PyCell {
1535+
#[pyslot]
1536+
fn slot_richcompare(
1537+
zelf: &PyObject,
1538+
other: &PyObject,
1539+
op: PyComparisonOp,
1540+
vm: &VirtualMachine,
1541+
) -> PyResult<Either<PyObjectRef, PyComparisonValue>> {
1542+
let (Some(zelf), Some(other)) = (zelf.downcast_ref::<Self>(), other.downcast_ref::<Self>())
1543+
else {
1544+
return Ok(Either::B(PyComparisonValue::NotImplemented));
1545+
};
1546+
// compare cells by contents; empty cells come before anything else
1547+
match (zelf.get(), other.get()) {
1548+
(Some(a), Some(b)) => a.rich_compare(b, op, vm).map(Either::A),
1549+
(a, b) => Ok(Either::B(op.eval_ord(b.is_none().cmp(&a.is_none())).into())),
1550+
}
1551+
}
1552+
15351553
pub(crate) const fn new(contents: Option<PyObjectRef>) -> Self {
15361554
Self {
15371555
contents: PyMutex::new(contents),
@@ -1561,6 +1579,30 @@ impl PyCell {
15611579
}
15621580
}
15631581

1582+
impl Representable for PyCell {
1583+
#[inline]
1584+
fn repr_str(zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<String> {
1585+
let id = zelf.get_id();
1586+
Ok(match zelf.get() {
1587+
Some(value) => {
1588+
let type_name = value.class().slot_name();
1589+
// CPython renders the type name with "%.80s", which reads at
1590+
// most 80 bytes and drops a character left incomplete by the cut.
1591+
let mut end = type_name.len().min(80);
1592+
while !type_name.is_char_boundary(end) {
1593+
end -= 1;
1594+
}
1595+
format!(
1596+
"<cell at {id:#x}: {} object at {:#x}>",
1597+
&type_name[..end],
1598+
value.get_id()
1599+
)
1600+
}
1601+
None => format!("<cell at {id:#x}: empty>"),
1602+
})
1603+
}
1604+
}
1605+
15641606
/// Vectorcall implementation for PyFunction (PEP 590).
15651607
/// Takes owned args to avoid cloning when filling fastlocals.
15661608
pub(crate) fn vectorcall_function(

0 commit comments

Comments
 (0)