-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-31333: Re-implement ABCMeta in C #5273
Changes from 1 commit
5c34508
cb7ffcf
b83ee80
181e83f
c084a7f
a192d5d
35a2472
4812450
b9038e2
bbee578
a3464fd
947bf7d
7ffc59e
41287a7
569cc44
34665a8
576acac
30098b4
51ede5d
5263e1a
1f7aee9
11fea70
7ff3fbb
9b4eb2f
ab20a33
39f2692
493d0ec
2fe2c54
9476af6
ab68cdb
3eb0a60
ed36b76
25fc5b9
b2f75b9
cdb5cdf
a1a3a52
a66b08c
86af9ae
b22232a
e51c5ca
bac7a43
4571649
0d7513b
c429f49
357b56d
cd80fcb
34e13c3
c5633b6
3cbbc12
23bcb07
0aab479
86e0660
c55e482
5f9526a
8174b61
bb8d623
ef59e54
22699fe
95cbf34
4d596cc
fa3cba3
6f18293
9100891
36c5643
dd2abda
99d950c
3762d49
404d1ce
0dc5fae
287b26a
ef34364
d4d78a1
f58822e
3b74bdc
db1c852
6e62be7
5384726
a48eecc
16a8db1
5ad3ea8
86a9b8d
36c2013
09c5370
207d8e9
a15377b
d31da13
eaff1cb
48de70e
3bd0666
702347a
e0c978b
b370dfe
a1ae0a7
4746211
001b416
fc528df
289c414
ac0c639
079e3be
9c49e5a
4146588
c133605
f82e04d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,50 +39,24 @@ typedef struct { | |
static void | ||
abc_data_dealloc(_abc_data *self) | ||
{ | ||
Py_DECREF(self->_abc_registry); | ||
Py_DECREF(self->_abc_cache); | ||
Py_DECREF(self->_abc_negative_cache); | ||
Py_XDECREF(self->_abc_registry); | ||
Py_XDECREF(self->_abc_cache); | ||
Py_XDECREF(self->_abc_negative_cache); | ||
Py_DECREF(self->_abc_negative_cache_version); | ||
Py_TYPE(self)->tp_free(self); | ||
} | ||
|
||
static PyObject * | ||
abc_data_new(PyTypeObject *type, PyObject *args, PyObject *kwds) | ||
{ | ||
PyObject *registry = NULL, *cache = NULL, *negative_cache = NULL; | ||
registry = PySet_New(NULL); | ||
if (registry == NULL) { | ||
return NULL; | ||
} | ||
cache = PySet_New(NULL); | ||
if (cache == NULL) { | ||
Py_DECREF(registry); | ||
return NULL; | ||
} | ||
negative_cache = PySet_New(NULL); | ||
if (negative_cache == NULL) { | ||
Py_DECREF(registry); | ||
Py_DECREF(cache); | ||
return NULL; | ||
} | ||
|
||
_abc_data *self = (_abc_data *) type->tp_alloc(type, 0); | ||
if (self == NULL) { | ||
goto error; | ||
return NULL; | ||
} | ||
|
||
self->_abc_registry = registry; | ||
self->_abc_cache = cache; | ||
self->_abc_negative_cache = negative_cache; | ||
self->_abc_negative_cache_version = abc_invalidation_counter; | ||
Py_INCREF(abc_invalidation_counter); | ||
return (PyObject *) self; | ||
|
||
error: | ||
Py_DECREF(registry); | ||
Py_DECREF(cache); | ||
Py_DECREF(negative_cache); | ||
return NULL; | ||
} | ||
|
||
PyDoc_STRVAR(abc_data_doc, | ||
|
@@ -116,7 +90,7 @@ _get_impl(PyObject *self) | |
static int | ||
_in_weak_set(PyObject *set, PyObject *obj) | ||
{ | ||
if (PySet_Size(set) == 0) { | ||
if (set == NULL || PySet_Size(set) == 0) { | ||
return 0; | ||
} | ||
PyObject *ref = PyWeakref_NewRef(obj, NULL); | ||
|
@@ -154,8 +128,16 @@ static PyMethodDef _destroy_def = { | |
}; | ||
|
||
static int | ||
_add_to_weak_set(PyObject *set, PyObject *obj) | ||
_add_to_weak_set(PyObject **pset, PyObject *obj) | ||
{ | ||
if (*pset == NULL) { | ||
*pset = PySet_New(NULL); | ||
if (*pset == NULL) { | ||
return -1; | ||
} | ||
} | ||
|
||
PyObject *set = *pset; | ||
PyObject *ref, *wr; | ||
PyObject *destroy_cb; | ||
wr = PyWeakref_NewRef(set, NULL); | ||
|
@@ -192,7 +174,7 @@ _reset_registry(PyObject *m, PyObject *args) | |
if (impl == NULL) { | ||
return NULL; | ||
} | ||
if (PySet_Clear(impl->_abc_registry) < 0) { | ||
if (impl->_abc_registry != NULL && PySet_Clear(impl->_abc_registry) < 0) { | ||
Py_DECREF(impl); | ||
return NULL; | ||
} | ||
|
@@ -465,7 +447,7 @@ _abc_register(PyObject *m, PyObject *args) | |
if (impl == NULL) { | ||
return NULL; | ||
} | ||
if (_add_to_weak_set(impl->_abc_registry, subclass) < 0) { | ||
if (_add_to_weak_set(&impl->_abc_registry, subclass) < 0) { | ||
Py_DECREF(impl); | ||
return NULL; | ||
} | ||
|
@@ -594,7 +576,8 @@ _abc_subclasscheck(PyObject *m, PyObject *args) | |
assert(r >= 0); // Both should be PyLong | ||
if (r > 0) { | ||
/* Invalidate the negative cache. */ | ||
if (PySet_Clear(impl->_abc_negative_cache) < 0) { | ||
if (impl->_abc_negative_cache != NULL && | ||
PySet_Clear(impl->_abc_negative_cache) < 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PEP 7 requires |
||
goto end; | ||
} | ||
/* INCREF the new value of cache version, | ||
|
@@ -621,15 +604,15 @@ _abc_subclasscheck(PyObject *m, PyObject *args) | |
} | ||
if (ok == Py_True) { | ||
Py_DECREF(ok); | ||
if (_add_to_weak_set(impl->_abc_cache, subclass) < 0) { | ||
if (_add_to_weak_set(&impl->_abc_cache, subclass) < 0) { | ||
goto end; | ||
} | ||
result = Py_True; | ||
goto end; | ||
} | ||
if (ok == Py_False) { | ||
Py_DECREF(ok); | ||
if (_add_to_weak_set(impl->_abc_negative_cache, subclass) < 0) { | ||
if (_add_to_weak_set(&impl->_abc_negative_cache, subclass) < 0) { | ||
goto end; | ||
} | ||
result = Py_False; | ||
|
@@ -652,7 +635,7 @@ _abc_subclasscheck(PyObject *m, PyObject *args) | |
goto end; | ||
} | ||
if ((PyObject *)self == mro_item) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Python version compares values, not just references. I don't know whether this matters. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is something we shouldn't worry about. Moreover, I actually think it is better if |
||
if (_add_to_weak_set(impl->_abc_cache, subclass) < 0) { | ||
if (_add_to_weak_set(&impl->_abc_cache, subclass) < 0) { | ||
goto end; | ||
} | ||
result = Py_True; | ||
|
@@ -675,7 +658,7 @@ _abc_subclasscheck(PyObject *m, PyObject *args) | |
for (pos = 0; pos < PyList_GET_SIZE(subclasses); pos++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessarily a list; |
||
int r = PyObject_IsSubclass(subclass, PyList_GET_ITEM(subclasses, pos)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, but the list itself holds strong references to all its elements, and will not be destroyed since we hold a strong reference to the list. Anyway, if you think it is important I can add an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, but note that this can happen only if a subclass overrides |
||
if (r > 0) { | ||
if (_add_to_weak_set(impl->_abc_cache, subclass) < 0) { | ||
if (_add_to_weak_set(&impl->_abc_cache, subclass) < 0) { | ||
goto end; | ||
} | ||
result = Py_True; | ||
|
@@ -687,7 +670,7 @@ _abc_subclasscheck(PyObject *m, PyObject *args) | |
} | ||
|
||
/* No dice; update negative cache. */ | ||
if (_add_to_weak_set(impl->_abc_negative_cache, subclass) < 0) { | ||
if (_add_to_weak_set(&impl->_abc_negative_cache, subclass) < 0) { | ||
goto end; | ||
} | ||
result = Py_False; | ||
|
@@ -715,6 +698,9 @@ subclasscheck_check_registry(_abc_data *impl, PyObject *subclass, | |
return 1; | ||
} | ||
|
||
if (impl->_abc_registry == NULL) { | ||
return 0; | ||
} | ||
Py_ssize_t registry_size = PySet_Size(impl->_abc_registry); | ||
if (registry_size == 0) { | ||
return 0; | ||
|
@@ -751,7 +737,7 @@ subclasscheck_check_registry(_abc_data *impl, PyObject *subclass, | |
break; | ||
} | ||
if (r > 0) { | ||
if (_add_to_weak_set(impl->_abc_cache, subclass) < 0) { | ||
if (_add_to_weak_set(&impl->_abc_cache, subclass) < 0) { | ||
ret = -1; | ||
break; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use
PySet_GET_SIZE()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, these attributes are not accessible from Python code so that we know that they always refer to sets.