Import test_isinstance + test_decorators, fix dict tuple key lookup

jgarzik · claude · jgarzik · commit 52cacfe8bfc9 · 2026-03-15T15:37:12.000Z
New tests:
- test_isinstance.py: 11 tests (8 pass, 3 skipped)
- test_decorators.py: 10 tests (all pass)

Bug fixes:
- Fix dict_keys_equal to use tp_richcompare fallback for non-string
  heap pointer keys (tuple keys, frozenset keys, etc.). Previously
  only handled string and numeric key equality, so tuple dict keys
  like d[(2,)] would fail to match even when present.
- Update bugs.md with fix #20 and updated known bugs list

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/Makefile b/Makefile
@@ -82,6 +82,10 @@ gen-cpython-tests:
 	@$(PYTHON) -m py_compile tests/cpython/test_dict.py
 	@echo "Compiling tests/cpython/test_set.py..."
 	@$(PYTHON) -m py_compile tests/cpython/test_set.py
+	@echo "Compiling tests/cpython/test_isinstance.py..."
+	@$(PYTHON) -m py_compile tests/cpython/test_isinstance.py
+	@echo "Compiling tests/cpython/test_decorators.py..."
+	@$(PYTHON) -m py_compile tests/cpython/test_decorators.py
 	@echo "Done."
 
 check-cpython: $(TARGET) gen-cpython-tests
@@ -111,3 +115,7 @@ check-cpython: $(TARGET) gen-cpython-tests
 	@./apython tests/cpython/__pycache__/test_dict.cpython-312.pyc
 	@echo "Running CPython test_set.py..."
 	@./apython tests/cpython/__pycache__/test_set.cpython-312.pyc
+	@echo "Running CPython test_isinstance.py..."
+	@./apython tests/cpython/__pycache__/test_isinstance.cpython-312.pyc
+	@echo "Running CPython test_decorators.py..."
+	@./apython tests/cpython/__pycache__/test_decorators.cpython-312.pyc
diff --git a/bugs.md b/bugs.md
@@ -96,13 +96,22 @@
 **Root cause**: Method not implemented, not registered in set tp_dict
 **Fix**: Implement set_method_update, register in init_builtin_methods
 
+### 20. dict_keys_equal only handles strings/numerics (dict.asm)
+**Symptom**: `d[(2,)]` fails to find key even though `(2,)` is in dict (memoize pattern broken)
+**Root cause**: `dict_keys_equal` returned "not equal" for any non-string, non-numeric heap pointers. Tuple keys, frozenset keys, etc. all failed equality checks.
+**Fix**: Add `.dke_try_richcompare` fallback that calls `tp_richcompare(PY_EQ)` + `obj_is_true` for non-string heap pointer keys.
+
 ### Known Bugs Not Yet Fixed
 - `dict.update(x=1, y=2)` with kwargs segfaults (methods.asm)
 - `repr(d.keys())` returns wrong value (dict view repr not implemented)
 - `(1,1) in d.items()` fails (dict_items __contains__ not implemented)
 - `tuple(t) is t` identity optimization not implemented
 - `list.append()` no-args segfaults (method arg count validation missing)
 - `assertRaises(fn)` double-free crash (exception handling memory issue)
+- `issubclass(C, (C,))` always returns False (tuple arg not supported)
+- `isinstance(1, 1)` doesn't raise TypeError (input validation missing)
+- `issubclass(1, int)` segfaults (input validation missing)
+- `func.__name__ = "x"` silently ignored (attribute set on functions not supported)
 
 ## New Infrastructure Added
 - `list_copy()` - standalone shallow copy function
diff --git a/src/pyo/dict.asm b/src/pyo/dict.asm
@@ -249,11 +249,11 @@ DEF_FUNC_LOCAL dict_keys_equal
     mov rax, [rbx + PyObject.ob_type]
     lea rcx, [rel str_type]
     cmp rax, rcx
-    jne .dke_ne_pop
+    jne .dke_try_richcompare
 
     mov rax, [r12 + PyObject.ob_type]
     cmp rax, rcx
-    jne .dke_ne_pop
+    jne .dke_try_richcompare
 
     ; Both strings — compare data
     lea rdi, [rbx + PyStrObject.data]
@@ -269,6 +269,41 @@ DEF_FUNC_LOCAL dict_keys_equal
     leave
     ret
 
+.dke_try_richcompare:
+    ; Both heap ptrs, not strings — try tp_richcompare
+    extern obj_is_true
+    mov rax, [rbx + PyObject.ob_type]
+    mov rax, [rax + PyTypeObject.tp_richcompare]
+    test rax, rax
+    jz .dke_ne_pop
+    ; Call tp_richcompare(a, b, PY_EQ, a_tag=TAG_PTR, b_tag=TAG_PTR)
+    mov rdi, rbx
+    mov rsi, r12
+    mov edx, PY_EQ
+    mov ecx, TAG_PTR
+    mov r8d, TAG_PTR
+    call rax
+    ; Check result: if NULL/TAG_NULL → not equal
+    test edx, edx
+    jz .dke_ne_pop
+    ; Check if result is truthy
+    mov rdi, rax
+    mov rsi, rdx
+    push rax
+    push rdx
+    call obj_is_true
+    mov ebx, eax           ; save truthiness
+    pop rdx
+    pop rdi
+    push rbx
+    mov rsi, rdx
+    DECREF_VAL rdi, rsi
+    pop rax                 ; truthiness result
+    pop r12
+    pop rbx
+    leave
+    ret
+
 .dke_ne_pop:
     xor eax, eax
     pop r12
diff --git a/tests/cpython/test_decorators.py b/tests/cpython/test_decorators.py
@@ -0,0 +1,180 @@
+"""CPython test_decorators.py adapted for apython."""
+import unittest
+
+
+def countcalls(counts):
+    "Decorator to count calls to a function"
+    def decorate(func):
+        func_name = func.__name__
+        counts[func_name] = 0
+        def call(*args, **kwds):
+            counts[func_name] += 1
+            return func(*args, **kwds)
+        call.__name__ = func_name
+        return call
+    return decorate
+
+
+def memoize(func):
+    saved = {}
+    def call(*args):
+        try:
+            return saved[args]
+        except KeyError:
+            res = func(*args)
+            saved[args] = res
+            return res
+    call.__name__ = func.__name__
+    return call
+
+
+class TestDecorators(unittest.TestCase):
+
+    def test_single_staticmethod(self):
+        class C(object):
+            @staticmethod
+            def foo(): return 42
+        self.assertEqual(C.foo(), 42)
+        self.assertEqual(C().foo(), 42)
+
+    def test_double(self):
+        class C(object):
+            @staticmethod
+            def foo(): return 42
+        self.assertEqual(C.foo(), 42)
+
+    def test_countcalls(self):
+        counts = {}
+
+        @countcalls(counts)
+        def double(x):
+            return x * 2
+
+        # Skip __name__ check - func.__name__ set not supported
+        self.assertEqual(double(2), 4)
+        self.assertEqual(counts['double'], 1)
+        self.assertEqual(double(3), 6)
+        self.assertEqual(counts['double'], 2)
+
+    def test_memoize(self):
+        counts = [0]
+
+        saved = {}
+        def memoize_local(func):
+            def call(*args):
+                try:
+                    return saved[args]
+                except KeyError:
+                    res = func(*args)
+                    saved[args] = res
+                    return res
+            return call
+
+        @memoize_local
+        def double(x):
+            counts[0] += 1
+            return x * 2
+
+        self.assertEqual(double(2), 4)
+        self.assertEqual(double(3), 6)
+        self.assertEqual(counts[0], 2)
+        # Cached results
+        self.assertEqual(double(2), 4)
+        self.assertEqual(double(3), 6)
+        self.assertEqual(counts[0], 2)  # no additional calls
+
+    def test_decorator_with_args(self):
+        def add_tag(tag):
+            def decorator(func):
+                def wrapper(*args, **kwargs):
+                    return tag + ": " + str(func(*args, **kwargs))
+                return wrapper
+            return decorator
+
+        @add_tag("result")
+        def compute(x):
+            return x * 10
+
+        self.assertEqual(compute(5), "result: 50")
+
+    def test_stacked_decorators(self):
+        log = []
+
+        def decorator_a(func):
+            def wrapper(*args):
+                log.append('a')
+                return func(*args)
+            return wrapper
+
+        def decorator_b(func):
+            def wrapper(*args):
+                log.append('b')
+                return func(*args)
+            return wrapper
+
+        @decorator_a
+        @decorator_b
+        def greet(name):
+            return "hello " + name
+
+        result = greet("world")
+        self.assertEqual(result, "hello world")
+        self.assertEqual(log, ['a', 'b'])
+
+    def test_classmethod_decorator(self):
+        class C:
+            count = 0
+
+            @classmethod
+            def increment(cls):
+                cls.count += 1
+                return cls.count
+
+        self.assertEqual(C.increment(), 1)
+        self.assertEqual(C.increment(), 2)
+        self.assertEqual(C.count, 2)
+
+    def test_property_decorator(self):
+        class C:
+            def __init__(self):
+                self._x = 0
+
+            @property
+            def x(self):
+                return self._x
+
+        c = C()
+        self.assertEqual(c.x, 0)
+        c._x = 42
+        self.assertEqual(c.x, 42)
+
+
+class TestClassDecorators(unittest.TestCase):
+
+    def test_simple(self):
+        def d(cls):
+            cls.decorated = True
+            return cls
+
+        @d
+        class C:
+            pass
+
+        self.assertTrue(C.decorated)
+
+    def test_with_args(self):
+        def d(tag):
+            def decorator(cls):
+                cls.tag = tag
+                return cls
+            return decorator
+
+        @d("hello")
+        class C:
+            pass
+
+        self.assertEqual(C.tag, "hello")
+
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/tests/cpython/test_isinstance.py b/tests/cpython/test_isinstance.py
@@ -0,0 +1,102 @@
+"""CPython test_isinstance.py adapted for apython."""
+import unittest
+
+
+# Normal classes
+class Super:
+    pass
+
+class Child(Super):
+    pass
+
+
+class TestIsInstanceIsSubclass(unittest.TestCase):
+
+    def test_isinstance_normal(self):
+        # normal instances
+        self.assertEqual(True, isinstance(Super(), Super))
+        self.assertEqual(False, isinstance(Super(), Child))
+
+        self.assertEqual(True, isinstance(Child(), Super))
+        self.assertEqual(True, isinstance(Child(), Child))
+
+    def test_isinstance_builtin(self):
+        self.assertTrue(isinstance(1, int))
+        self.assertTrue(isinstance(1.0, float))
+        self.assertTrue(isinstance("hello", str))
+        self.assertTrue(isinstance([], list))
+        self.assertTrue(isinstance({}, dict))
+        self.assertTrue(isinstance((), tuple))
+        self.assertTrue(isinstance(True, bool))
+        self.assertTrue(isinstance(True, int))  # bool is subclass of int
+        self.assertFalse(isinstance(1, str))
+        self.assertFalse(isinstance("hello", int))
+
+    def test_isinstance_tuple_arg(self):
+        self.assertTrue(isinstance(1, (int, str)))
+        self.assertTrue(isinstance("a", (int, str)))
+        self.assertFalse(isinstance(1.0, (int, str)))
+
+    def test_isinstance_none(self):
+        self.assertFalse(isinstance(None, int))
+        self.assertFalse(isinstance(None, str))
+        self.assertFalse(isinstance(None, list))
+
+    def test_subclass_normal(self):
+        # normal classes
+        self.assertEqual(True, issubclass(Super, Super))
+        self.assertEqual(False, issubclass(Super, Child))
+
+        self.assertEqual(True, issubclass(Child, Child))
+        self.assertEqual(True, issubclass(Child, Super))
+
+    def test_subclass_builtin(self):
+        self.assertTrue(issubclass(bool, int))
+        self.assertTrue(issubclass(int, int))
+        self.assertFalse(issubclass(int, str))
+        self.assertFalse(issubclass(str, int))
+
+    @unittest.skip("issubclass with tuple arg always returns False")
+    def test_subclass_tuple(self):
+        pass
+
+    def test_isinstance_with_custom_class(self):
+        class A:
+            pass
+        class B(A):
+            pass
+        class C(B):
+            pass
+
+        self.assertTrue(isinstance(C(), A))
+        self.assertTrue(isinstance(C(), B))
+        self.assertTrue(isinstance(C(), C))
+        self.assertFalse(isinstance(A(), B))
+        self.assertFalse(isinstance(A(), C))
+
+    def test_issubclass_with_custom_class(self):
+        class A:
+            pass
+        class B(A):
+            pass
+        class C(B):
+            pass
+
+        self.assertTrue(issubclass(C, A))
+        self.assertTrue(issubclass(C, B))
+        self.assertTrue(issubclass(C, C))
+        self.assertTrue(issubclass(B, A))
+        self.assertFalse(issubclass(A, B))
+        self.assertFalse(issubclass(A, C))
+
+    @unittest.skip("isinstance(1,1) doesn't raise TypeError yet")
+    def test_isinstance_errors(self):
+        pass
+
+    @unittest.skip("issubclass(1,int) segfaults")
+    def test_issubclass_errors(self):
+        pass
+
+
+if __name__ == "__main__":
+    unittest.main()
