Skip to content

Commit abf4bfe

Browse files
dolfinusdolfinus
committed
gh-92810: Return __subclasses__clause back
1 parent 7960480 commit abf4bfe

File tree

4 files changed

+222
-50
lines changed

4 files changed

+222
-50
lines changed

‎Lib/_py_abc.py

+24-16
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from _weakrefset import WeakSet
2+
from weakref import WeakKeyDictionary
23

34

45
def get_cache_token():
@@ -31,6 +32,7 @@ class ABCMeta(type):
3132
# Note: this counter is private. Use `abc.get_cache_token()` for
3233
# external code.
3334
_abc_invalidation_counter = 0
35+
_abc_issubclass_context = WeakKeyDictionary()
3436

3537
def __new__(mcls, name, bases, namespace, /, **kwargs):
3638
cls = super().__new__(mcls, name, bases, namespace, **kwargs)
@@ -65,21 +67,7 @@ def register(cls, subclass):
6567
if issubclass(cls, subclass):
6668
# This would create a cycle, which is bad for the algorithm below
6769
raise RuntimeError("Refusing to create an inheritance cycle")
68-
69-
# Actual registration
7070
cls._abc_registry.add(subclass)
71-
72-
# Recursively register the subclass in all ABC bases, to avoid recursive lookups.
73-
# >>> class Ancestor1(ABC): pass
74-
# >>> class Ancestor2(Ancestor1): pass
75-
# >>> class Other: pass
76-
# >>> Ancestor2.register(Other) # same result for Ancestor1.register(Other)
77-
# >>> issubclass(Other, Ancestor2) is True
78-
# >>> issubclass(Other, Ancestor1) is True
79-
for pcls in cls.__mro__:
80-
if hasattr(pcls, "_abc_registry"):
81-
pcls._abc_registry.add(subclass)
82-
8371
# Invalidate negative cache
8472
ABCMeta._abc_invalidation_counter += 1
8573
return subclass
@@ -152,6 +140,26 @@ def __subclasscheck__(cls, subclass):
152140
if issubclass(subclass, rcls):
153141
cls._abc_cache.add(subclass)
154142
return True
155-
# No dice; update negative cache
156-
cls._abc_negative_cache.add(subclass)
143+
144+
# Check if it's a subclass of a subclass (recursive)
145+
for scls in cls.__subclasses__():
146+
# If inside recursive issubclass check, avoid adding classes to any cache because this
147+
# may drastically increase memory usage.
148+
# Unfortunately, issubclass/__subclasscheck__ don't accept third argument with context,
149+
# so using global context within ABCMeta.
150+
# This is done only on first method call, others will use cached result.
151+
scls_context = ABCMeta._abc_issubclass_context.setdefault(scls, WeakSet())
152+
try:
153+
scls_context.add(cls)
154+
result = issubclass(subclass, scls)
155+
finally:
156+
scls_context.remove(cls)
157+
158+
if result:
159+
if not ABCMeta._abc_issubclass_context.get(cls, None):
160+
cls._abc_cache.add(subclass)
161+
return True
162+
163+
if not ABCMeta._abc_issubclass_context.get(cls, None):
164+
cls._abc_negative_cache.add(subclass)
157165
return False

‎Lib/test/test_abc.py

+166-31
Original file line numberDiff line numberDiff line change
@@ -270,29 +270,100 @@ def x(self):
270270
class C(metaclass=meta):
271271
pass
272272

273+
def test_isinstance_direct_inheritance(self):
274+
class A(metaclass=abc_ABCMeta):
275+
pass
276+
class B(A):
277+
pass
278+
class C(A):
279+
pass
280+
a = A()
281+
b = B()
282+
c = C()
283+
# trigger caching
284+
for _ in range(2):
285+
self.assertIsInstance(a, A)
286+
self.assertIsInstance(a, (A,))
287+
self.assertNotIsInstance(a, B)
288+
self.assertNotIsInstance(a, (B,))
289+
self.assertNotIsInstance(a, C)
290+
self.assertNotIsInstance(a, (C,))
291+
292+
self.assertIsInstance(b, B)
293+
self.assertIsInstance(b, (B,))
294+
self.assertIsInstance(b, A)
295+
self.assertIsInstance(b, (A,))
296+
self.assertNotIsInstance(b, C)
297+
self.assertNotIsInstance(b, (C,))
298+
299+
self.assertIsInstance(c, C)
300+
self.assertIsInstance(c, (C,))
301+
self.assertIsInstance(c, A)
302+
self.assertIsInstance(c, (A,))
303+
self.assertNotIsInstance(c, B)
304+
self.assertNotIsInstance(c, (B,))
305+
306+
self.assertIsSubclass(B, A)
307+
self.assertIsSubclass(B, (A,))
308+
self.assertIsSubclass(C, A)
309+
self.assertIsSubclass(C, (A,))
310+
self.assertNotIsSubclass(B, C)
311+
self.assertNotIsSubclass(B, (C,))
312+
self.assertNotIsSubclass(C, B)
313+
self.assertNotIsSubclass(C, (B,))
314+
self.assertNotIsSubclass(A, B)
315+
self.assertNotIsSubclass(A, (B,))
316+
self.assertNotIsSubclass(A, C)
317+
self.assertNotIsSubclass(A, (C,))
318+
273319
def test_registration_basics(self):
274320
class A(metaclass=abc_ABCMeta):
275321
pass
276322
class B(object):
277323
pass
324+
a = A()
278325
b = B()
279-
self.assertNotIsSubclass(B, A)
280-
self.assertNotIsSubclass(B, (A,))
281-
self.assertNotIsInstance(b, A)
282-
self.assertNotIsInstance(b, (A,))
326+
327+
# trigger caching
328+
for _ in range(2):
329+
self.assertNotIsSubclass(B, A)
330+
self.assertNotIsSubclass(B, (A,))
331+
self.assertNotIsInstance(b, A)
332+
self.assertNotIsInstance(b, (A,))
333+
334+
self.assertNotIsSubclass(A, B)
335+
self.assertNotIsSubclass(A, (B,))
336+
self.assertNotIsInstance(a, B)
337+
self.assertNotIsInstance(a, (B,))
338+
283339
B1 = A.register(B)
284-
self.assertIsSubclass(B, A)
285-
self.assertIsSubclass(B, (A,))
286-
self.assertIsInstance(b, A)
287-
self.assertIsInstance(b, (A,))
288-
self.assertIs(B1, B)
340+
# trigger caching
341+
for _ in range(2):
342+
self.assertIsSubclass(B, A)
343+
self.assertIsSubclass(B, (A,))
344+
self.assertIsInstance(b, A)
345+
self.assertIsInstance(b, (A,))
346+
self.assertIs(B1, B)
347+
348+
self.assertNotIsSubclass(A, B)
349+
self.assertNotIsSubclass(A, (B,))
350+
self.assertNotIsInstance(a, B)
351+
self.assertNotIsInstance(a, (B,))
352+
289353
class C(B):
290354
pass
291355
c = C()
292-
self.assertIsSubclass(C, A)
293-
self.assertIsSubclass(C, (A,))
294-
self.assertIsInstance(c, A)
295-
self.assertIsInstance(c, (A,))
356+
# trigger caching
357+
for _ in range(2):
358+
self.assertIsSubclass(C, A)
359+
self.assertIsSubclass(C, (A,))
360+
self.assertIsInstance(c, A)
361+
self.assertIsInstance(c, (A,))
362+
363+
self.assertNotIsSubclass(A, C)
364+
self.assertNotIsSubclass(A, (C,))
365+
self.assertNotIsInstance(a, C)
366+
self.assertNotIsInstance(a, (C,))
296367

297368
def test_register_as_class_deco(self):
298369
class A(metaclass=abc_ABCMeta):
@@ -377,41 +448,75 @@ class A(metaclass=abc_ABCMeta):
377448
pass
378449
self.assertIsSubclass(A, A)
379450
self.assertIsSubclass(A, (A,))
451+
380452
class B(metaclass=abc_ABCMeta):
381453
pass
382454
self.assertNotIsSubclass(A, B)
383455
self.assertNotIsSubclass(A, (B,))
384456
self.assertNotIsSubclass(B, A)
385457
self.assertNotIsSubclass(B, (A,))
458+
386459
class C(metaclass=abc_ABCMeta):
387460
pass
388461
A.register(B)
389462
class B1(B):
390463
pass
391-
self.assertIsSubclass(B1, A)
392-
self.assertIsSubclass(B1, (A,))
464+
# trigger caching
465+
for _ in range(2):
466+
self.assertIsSubclass(B1, A)
467+
self.assertIsSubclass(B1, (A,))
468+
393469
class C1(C):
394470
pass
395471
B1.register(C1)
396-
self.assertNotIsSubclass(C, B)
397-
self.assertNotIsSubclass(C, (B,))
398-
self.assertNotIsSubclass(C, B1)
399-
self.assertNotIsSubclass(C, (B1,))
400-
self.assertIsSubclass(C1, A)
401-
self.assertIsSubclass(C1, (A,))
402-
self.assertIsSubclass(C1, B)
403-
self.assertIsSubclass(C1, (B,))
404-
self.assertIsSubclass(C1, B1)
405-
self.assertIsSubclass(C1, (B1,))
472+
# trigger caching
473+
for _ in range(2):
474+
self.assertNotIsSubclass(C, B)
475+
self.assertNotIsSubclass(C, (B,))
476+
self.assertNotIsSubclass(C, B1)
477+
self.assertNotIsSubclass(C, (B1,))
478+
self.assertIsSubclass(C1, A)
479+
self.assertIsSubclass(C1, (A,))
480+
self.assertIsSubclass(C1, B)
481+
self.assertIsSubclass(C1, (B,))
482+
self.assertIsSubclass(C1, B1)
483+
self.assertIsSubclass(C1, (B1,))
484+
406485
C1.register(int)
407486
class MyInt(int):
408487
pass
409-
self.assertIsSubclass(MyInt, A)
410-
self.assertIsSubclass(MyInt, (A,))
411-
self.assertIsInstance(42, A)
412-
self.assertIsInstance(42, (A,))
488+
# trigger caching
489+
for _ in range(2):
490+
self.assertIsSubclass(MyInt, A)
491+
self.assertIsSubclass(MyInt, (A,))
492+
self.assertIsInstance(42, A)
493+
self.assertIsInstance(42, (A,))
413494

414-
def test_subclasses_bad_arguments(self):
495+
def test_custom_subclasses(self):
496+
class A: pass
497+
class B: pass
498+
499+
class Parent1(metaclass=abc_ABCMeta):
500+
@classmethod
501+
def __subclasses__(cls):
502+
return [A]
503+
504+
class Parent2(metaclass=abc_ABCMeta):
505+
__subclasses__ = lambda: [A]
506+
507+
# trigger caching
508+
for _ in range(2):
509+
self.assertIsInstance(A(), Parent1)
510+
self.assertIsSubclass(A, Parent1)
511+
self.assertNotIsInstance(B(), Parent1)
512+
self.assertNotIsSubclass(B, Parent1)
513+
514+
self.assertIsInstance(A(), Parent2)
515+
self.assertIsSubclass(A, Parent2)
516+
self.assertNotIsInstance(B(), Parent2)
517+
self.assertNotIsSubclass(B, Parent2)
518+
519+
def test_issubclass_bad_arguments(self):
415520
class A(metaclass=abc_ABCMeta):
416521
pass
417522

@@ -429,6 +534,37 @@ class C:
429534
with self.assertRaises(TypeError):
430535
issubclass(C(), A)
431536

537+
# bpo-34441: Check that issubclass() doesn't crash on bogus
538+
# classes.
539+
bogus_subclasses = [
540+
None,
541+
lambda x: [],
542+
lambda: 42,
543+
lambda: [42],
544+
]
545+
546+
for i, func in enumerate(bogus_subclasses):
547+
class S(metaclass=abc_ABCMeta):
548+
__subclasses__ = func
549+
550+
with self.subTest(i=i):
551+
with self.assertRaises(TypeError):
552+
issubclass(int, S)
553+
554+
# Also check that issubclass() propagates exceptions raised by
555+
# __subclasses__.
556+
class CustomError(Exception): ...
557+
exc_msg = "exception from __subclasses__"
558+
559+
def raise_exc():
560+
raise CustomError(exc_msg)
561+
562+
class S(metaclass=abc_ABCMeta):
563+
__subclasses__ = raise_exc
564+
565+
with self.assertRaisesRegex(CustomError, exc_msg):
566+
issubclass(int, S)
567+
432568
def test_subclasshook(self):
433569
class A(metaclass=abc.ABCMeta):
434570
@classmethod
@@ -491,7 +627,6 @@ def foo(self):
491627
self.assertEqual(A.__abstractmethods__, set())
492628
A()
493629

494-
495630
def test_update_new_abstractmethods(self):
496631
class A(metaclass=abc_ABCMeta):
497632
@abc.abstractmethod

‎Lib/test/test_isinstance.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -351,13 +351,13 @@ class B:
351351
with support.infinite_recursion(25):
352352
self.assertRaises(RecursionError, issubclass, X(), int)
353353

354-
def test_override_subclasses(self):
354+
def test_custom_subclasses_are_ignored(self):
355355
class A: pass
356356
class B: pass
357357

358358
class Parent1:
359359
@classmethod
360-
def __subclasses__(self):
360+
def __subclasses__(cls):
361361
return [A, B]
362362

363363
class Parent2:

‎Modules/_abc.c

+30-1
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,7 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self,
763763

764764
PyObject *ok, *subclasses = NULL, *result = NULL;
765765
_abcmodule_state *state = NULL;
766+
Py_ssize_t pos;
766767
int incache;
767768
_abc_data *impl = _get_impl(module, self);
768769
if (impl == NULL) {
@@ -849,7 +850,35 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self,
849850
goto end;
850851
}
851852

852-
/* No dice; update negative cache. */
853+
/* 6. Check if it's a subclass of a subclass (recursive). */
854+
subclasses = PyObject_CallMethod(self, "__subclasses__", NULL);
855+
if (subclasses == NULL) {
856+
goto end;
857+
}
858+
if (!PyList_Check(subclasses)) {
859+
PyErr_SetString(PyExc_TypeError, "__subclasses__() must return a list");
860+
goto end;
861+
}
862+
for (pos = 0; pos < PyList_GET_SIZE(subclasses); pos++) {
863+
PyObject *scls = PyList_GetItemRef(subclasses, pos);
864+
if (scls == NULL) {
865+
goto end;
866+
}
867+
int r = PyObject_IsSubclass(subclass, scls);
868+
Py_DECREF(scls);
869+
if (r > 0) {
870+
if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) {
871+
goto end;
872+
}
873+
result = Py_True;
874+
goto end;
875+
}
876+
if (r < 0) {
877+
goto end;
878+
}
879+
}
880+
881+
/* Recursive calls lead to uncontrolled negative cache growth, avoid this */
853882
if (_add_to_weak_set(impl, &impl->_abc_negative_cache, subclass) < 0) {
854883
goto end;
855884
}

0 commit comments

Comments
 (0)