Skip to content

Commit b629fdd

Browse files
authored
GH-99298: Clean up attribute specializations (GH-99398)
1 parent 8555dee commit b629fdd

File tree

5 files changed

+39
-57
lines changed

5 files changed

+39
-57
lines changed

Include/internal/pycore_code.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,10 +213,10 @@ extern int _PyLineTable_PreviousAddressRange(PyCodeAddressRange *range);
213213

214214
/* Specialization functions */
215215

216-
extern int _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr,
217-
PyObject *name);
218-
extern int _Py_Specialize_StoreAttr(PyObject *owner, _Py_CODEUNIT *instr,
216+
extern void _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr,
219217
PyObject *name);
218+
extern void _Py_Specialize_StoreAttr(PyObject *owner, _Py_CODEUNIT *instr,
219+
PyObject *name);
220220
extern void _Py_Specialize_LoadGlobal(PyObject *globals, PyObject *builtins,
221221
_Py_CODEUNIT *instr, PyObject *name);
222222
extern void _Py_Specialize_BinarySubscr(PyObject *sub, PyObject *container,
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Remove the remaining error paths for attribute specializations, and refuse
2+
to specialize attribute accesses on types that haven't had
3+
:c:func:`PyType_Ready` called on them yet.

Python/bytecodes.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,11 +1137,7 @@ dummy_func(
11371137
PyObject *owner = TOP();
11381138
PyObject *name = GETITEM(names, oparg);
11391139
next_instr--;
1140-
if (_Py_Specialize_StoreAttr(owner, next_instr, name)) {
1141-
// "undo" the rewind so end up in the correct handler:
1142-
next_instr++;
1143-
goto error;
1144-
}
1140+
_Py_Specialize_StoreAttr(owner, next_instr, name);
11451141
DISPATCH_SAME_OPARG();
11461142
}
11471143
STAT_INC(STORE_ATTR, deferred);
@@ -1713,11 +1709,7 @@ dummy_func(
17131709
PyObject *owner = TOP();
17141710
PyObject *name = GETITEM(names, oparg>>1);
17151711
next_instr--;
1716-
if (_Py_Specialize_LoadAttr(owner, next_instr, name)) {
1717-
// "undo" the rewind so end up in the correct handler:
1718-
next_instr++;
1719-
goto error;
1720-
}
1712+
_Py_Specialize_LoadAttr(owner, next_instr, name);
17211713
DISPATCH_SAME_OPARG();
17221714
}
17231715
STAT_INC(LOAD_ATTR, deferred);

Python/generated_cases.c.h

Lines changed: 2 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/specialize.c

Lines changed: 29 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -663,32 +663,33 @@ static int specialize_attr_loadmethod(PyObject* owner, _Py_CODEUNIT* instr, PyOb
663663
PyObject* descr, DescriptorClassification kind);
664664
static int specialize_class_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject* name);
665665

666-
int
666+
void
667667
_Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name)
668668
{
669669
assert(_PyOpcode_Caches[LOAD_ATTR] == INLINE_CACHE_ENTRIES_LOAD_ATTR);
670670
_PyAttrCache *cache = (_PyAttrCache *)(instr + 1);
671+
PyTypeObject *type = Py_TYPE(owner);
672+
if (!_PyType_IsReady(type)) {
673+
// We *might* not really need this check, but we inherited it from
674+
// PyObject_GenericGetAttr and friends... and this way we still do the
675+
// right thing if someone forgets to call PyType_Ready(type):
676+
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER);
677+
goto fail;
678+
}
671679
if (PyModule_CheckExact(owner)) {
672-
int err = specialize_module_load_attr(owner, instr, name, LOAD_ATTR,
673-
LOAD_ATTR_MODULE);
674-
if (err) {
680+
if (specialize_module_load_attr(owner, instr, name, LOAD_ATTR,
681+
LOAD_ATTR_MODULE))
682+
{
675683
goto fail;
676684
}
677685
goto success;
678686
}
679687
if (PyType_Check(owner)) {
680-
int err = specialize_class_load_attr(owner, instr, name);
681-
if (err) {
688+
if (specialize_class_load_attr(owner, instr, name)) {
682689
goto fail;
683690
}
684691
goto success;
685692
}
686-
PyTypeObject *type = Py_TYPE(owner);
687-
if (type->tp_dict == NULL) {
688-
if (PyType_Ready(type) < 0) {
689-
return -1;
690-
}
691-
}
692693
PyObject *descr = NULL;
693694
DescriptorClassification kind = analyze_descriptor(type, name, &descr, 0);
694695
assert(descr != NULL || kind == ABSENT || kind == GETSET_OVERRIDDEN);
@@ -803,35 +804,36 @@ _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name)
803804
case ABSENT:
804805
break;
805806
}
806-
int err = specialize_dict_access(
807-
owner, instr, type, kind, name,
808-
LOAD_ATTR, LOAD_ATTR_INSTANCE_VALUE, LOAD_ATTR_WITH_HINT
809-
);
810-
if (err < 0) {
811-
return -1;
812-
}
813-
if (err) {
807+
if (specialize_dict_access(owner, instr, type, kind, name, LOAD_ATTR,
808+
LOAD_ATTR_INSTANCE_VALUE, LOAD_ATTR_WITH_HINT))
809+
{
814810
goto success;
815811
}
816812
fail:
817813
STAT_INC(LOAD_ATTR, failure);
818814
assert(!PyErr_Occurred());
819815
_Py_SET_OPCODE(*instr, LOAD_ATTR);
820816
cache->counter = adaptive_counter_backoff(cache->counter);
821-
return 0;
817+
return;
822818
success:
823819
STAT_INC(LOAD_ATTR, success);
824820
assert(!PyErr_Occurred());
825821
cache->counter = adaptive_counter_cooldown();
826-
return 0;
827822
}
828823

829-
int
824+
void
830825
_Py_Specialize_StoreAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name)
831826
{
832827
assert(_PyOpcode_Caches[STORE_ATTR] == INLINE_CACHE_ENTRIES_STORE_ATTR);
833828
_PyAttrCache *cache = (_PyAttrCache *)(instr + 1);
834829
PyTypeObject *type = Py_TYPE(owner);
830+
if (!_PyType_IsReady(type)) {
831+
// We *might* not really need this check, but we inherited it from
832+
// PyObject_GenericSetAttr and friends... and this way we still do the
833+
// right thing if someone forgets to call PyType_Ready(type):
834+
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER);
835+
goto fail;
836+
}
835837
if (PyModule_CheckExact(owner)) {
836838
SPECIALIZATION_FAIL(STORE_ATTR, SPEC_FAIL_OVERRIDDEN);
837839
goto fail;
@@ -890,28 +892,21 @@ _Py_Specialize_StoreAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name)
890892
case ABSENT:
891893
break;
892894
}
893-
894-
int err = specialize_dict_access(
895-
owner, instr, type, kind, name,
896-
STORE_ATTR, STORE_ATTR_INSTANCE_VALUE, STORE_ATTR_WITH_HINT
897-
);
898-
if (err < 0) {
899-
return -1;
900-
}
901-
if (err) {
895+
if (specialize_dict_access(owner, instr, type, kind, name, STORE_ATTR,
896+
STORE_ATTR_INSTANCE_VALUE, STORE_ATTR_WITH_HINT))
897+
{
902898
goto success;
903899
}
904900
fail:
905901
STAT_INC(STORE_ATTR, failure);
906902
assert(!PyErr_Occurred());
907903
_Py_SET_OPCODE(*instr, STORE_ATTR);
908904
cache->counter = adaptive_counter_backoff(cache->counter);
909-
return 0;
905+
return;
910906
success:
911907
STAT_INC(STORE_ATTR, success);
912908
assert(!PyErr_Occurred());
913909
cache->counter = adaptive_counter_cooldown();
914-
return 0;
915910
}
916911

917912

0 commit comments

Comments
 (0)