Skip to content

Commit 4c2eb69

Browse files
committed
Fix the buildbot test failure (I hope).
The problem was that sometimes a hint of -1 could be set, meaning the dict lookup failed. My code mistook that for a slot offset. The fix is simple, because slot offsets can never be 0, so ~offset can never be -1. I also cleaned up some comments and a variable name.
1 parent b725a65 commit 4c2eb69

File tree

1 file changed

+18
-16
lines changed

1 file changed

+18
-16
lines changed

Python/ceval.c

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3174,9 +3174,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
31743174
la = &co_opcache->u.la;
31753175
if (la->type == type && la->tp_version_tag == type->tp_version_tag)
31763176
{
3177-
// Hint >= 0 is a dict index; hint < 0 is an inverted slot index.
3178-
if (la->hint < 0) {
3179-
/* Even faster path -- slot hint */
3177+
// Hint >= 0 is a dict index; hint == -1 is a dict miss.
3178+
// Hint < -1 is an inverted slot offset (offsets are never 0).
3179+
if (la->hint < -1) {
3180+
// Even faster path -- slot hint.
31803181
Py_ssize_t offset = ~la->hint;
31813182
// fprintf(stderr, "Using hint for offset %zd\n", offset);
31823183
char *addr = (char *)owner + offset;
@@ -3190,7 +3191,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
31903191
// Else slot is NULL. Fall through to slow path to raise AttributeError(name).
31913192
// Don't DEOPT, since the slot is still there.
31923193
} else {
3193-
// Fast path for dict
3194+
// Fast path for dict.
31943195
assert(type->tp_dict != NULL);
31953196
assert(type->tp_dictoffset > 0);
31963197

@@ -3204,11 +3205,11 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
32043205

32053206
if (res != NULL) {
32063207
if (la->hint == hint && hint >= 0) {
3207-
/* Our hint has helped -- cache hit. */
3208+
// Our hint has helped -- cache hit.
32083209
OPCACHE_STAT_ATTR_HIT();
32093210
} else {
3210-
/* The hint we provided didn't work.
3211-
Maybe next time? */
3211+
// The hint we provided didn't work.
3212+
// Maybe next time?
32123213
OPCACHE_MAYBE_DEOPT_LOAD_ATTR();
32133214
}
32143215

@@ -3218,13 +3219,13 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
32183219
Py_DECREF(dict);
32193220
DISPATCH();
32203221
} else {
3221-
// This attribute can be missing sometimes -- we
3222-
// don't want to optimize this lookup.
3222+
// This attribute can be missing sometimes;
3223+
// we don't want to optimize this lookup.
32233224
OPCACHE_DEOPT_LOAD_ATTR();
32243225
Py_DECREF(dict);
32253226
}
32263227
} else {
3227-
// There is no dict, or __dict__ doesn't satisfy PyDict_CheckExact
3228+
// There is no dict, or __dict__ doesn't satisfy PyDict_CheckExact.
32283229
OPCACHE_DEOPT_LOAD_ATTR();
32293230
}
32303231
}
@@ -3236,7 +3237,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
32363237
OPCACHE_STAT_ATTR_MISS();
32373238
}
32383239

3239-
if (co_opcache != NULL && /* co_opcache can be NULL after a DEOPT() call. */
3240+
if (co_opcache != NULL && // co_opcache can be NULL after a DEOPT() call.
32403241
type->tp_getattro == PyObject_GenericGetAttr)
32413242
{
32423243
if (type->tp_dict == NULL) {
@@ -3255,6 +3256,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
32553256
struct PyMemberDef *dmem = member->d_member;
32563257
if (dmem->type == T_OBJECT_EX) {
32573258
Py_ssize_t offset = dmem->offset;
3259+
assert(offset > 0); // 0 would be confused with dict hint == -1 (miss).
32583260

32593261
if (co_opcache->optimized == 0) {
32603262
// First time we optimize this opcode.
@@ -3291,7 +3293,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
32913293
if (dict != NULL && PyDict_CheckExact(dict)) {
32923294
Py_INCREF(dict);
32933295
res = NULL;
3294-
Py_ssize_t ret = _PyDict_GetItemHint((PyDictObject*)dict, name, -1, &res);
3296+
Py_ssize_t hint = _PyDict_GetItemHint((PyDictObject*)dict, name, -1, &res);
32953297
if (res != NULL) {
32963298
Py_INCREF(res);
32973299
Py_DECREF(dict);
@@ -3307,25 +3309,25 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
33073309
la = &co_opcache->u.la;
33083310
la->type = type;
33093311
la->tp_version_tag = type->tp_version_tag;
3310-
la->hint = ret;
3312+
la->hint = hint;
33113313

33123314
DISPATCH();
33133315
}
33143316
Py_DECREF(dict);
33153317
} else {
3316-
// There is no dict, or __dict__ doesn't satisfy PyDict_CheckExact
3318+
// There is no dict, or __dict__ doesn't satisfy PyDict_CheckExact.
33173319
OPCACHE_DEOPT_LOAD_ATTR();
33183320
}
33193321
} else {
3320-
// The object's class does not have a tp_dictoffset we can use
3322+
// The object's class does not have a tp_dictoffset we can use.
33213323
OPCACHE_DEOPT_LOAD_ATTR();
33223324
}
33233325
} else if (type->tp_getattro != PyObject_GenericGetAttr) {
33243326
OPCACHE_DEOPT_LOAD_ATTR();
33253327
}
33263328
}
33273329

3328-
/* slow path */
3330+
// Slow path.
33293331
res = PyObject_GetAttr(owner, name);
33303332
Py_DECREF(owner);
33313333
SET_TOP(res);

0 commit comments

Comments
 (0)