8000 Address half of Mark's review · python/cpython@992731d · GitHub
[go: up one dir, main page]

Skip to content

Commit 992731d

Browse files
Address half of Mark's review
1 parent 2ff4f36 commit 992731d

File tree

9 files changed

+68
-63
lines changed

9 files changed

+68
-63
lines changed

Include/internal/pycore_frame.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ static inline _PyStackRef *_PyFrame_Stackbase(_PyInterpreterFrame *f) {
8585

8686
static inline _PyStackRef _PyFrame_StackPeek(_PyInterpreterFrame *f) {
8787
assert(f->stacktop > _PyFrame_GetCode(f)->co_nlocalsplus);
88-
assert(PyStackRef_AsPyObjectBorrow(f->localsplus[f->stacktop-1]) != NULL);
88+
assert(!PyStackRef_IsNull(f->localsplus[f->stacktop-1]));
8989
return f->localsplus[f->stacktop-1];
9090
}
9191

Include/internal/pycore_stackref.h

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,18 @@ PyStackRef_AsPyObjectSteal(_PyStackRef tagged)
120120
# define PyStackRef_AsPyObjectSteal(tagged) PyStackRef_AsPyObjectBorrow(tagged)
121121
#endif
122122

123+
#ifdef Py_GIL_DISABLED
124+
// Converts a PyStackRef back to a PyObject *, converting the
125+
// stackref to a new reference.
126+
static inline PyObject *
127+
PyStackRef_AsPyObjectNew(_PyStackRef tagged)
128+
{
129+
return Py_NewRef(PyStackRef_AsPyObjectBorrow(tagged));
130+
}
131+
#else
132+
# define PyStackRef_AsPyObjectNew(tagged) Py_NewRef(PyStackRef_AsPyObjectBorrow(tagged))
133+
#endif
134+
123135
static inline PyTypeObject *
124136
PyStackRef_TYPE(_PyStackRef stackref)
125137
{
@@ -165,14 +177,6 @@ PyStackRef_FromPyObjectNew(PyObject *obj)
165177
# define PyStackRef_FromPyObjectNew(obj) ((_PyStackRef){ .bits = (uintptr_t)(Py_NewRef(obj)) })
166178
#endif
167179

168-
#define PyStackRef_XSET(dst, src) \
169-
do { \
170-
_PyStackRef *_tmp_dst_ptr = &(dst); \
171-
_PyStackRef _tmp_old_dst = (*_tmp_dst_ptr); \
172-
*_tmp_dst_ptr = (src); \
173-
PyStackRef_XCLOSE(_tmp_old_dst); \
174-
} while (0)
175-
176180
#define PyStackRef_CLEAR(op) \
177181
do { \
178182
_PyStackRef *_tmp_op_ptr = &(op); \
@@ -220,6 +224,7 @@ PyStackRef_DUP(_PyStackRef tagged)
220224
return tagged;
221225
}
222226
#else
227+
// Needs to be macro to not overflow on WASI debug.
223228
# define PyStackRef_DUP(stackref) PyStackRef_FromPyObjectSteal(Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref)))
224229
#endif
225230

Objects/frameobject.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,8 @@ framelocalsproxy_setitem(PyObject *self, PyObject *key, PyObject *value)
171171
PyStackRef_XCLOSE(oldvalue);
172172
}
173173
} else if (value != PyStackRef_AsPyObjectBorrow(oldvalue)) {
174-
PyStackRef_XSET(fast[i], PyStackRef_FromPyObjectNew(value));
174+
PyStackRef_XCLOSE(fast[i]);
175+
fast[i] = PyStackRef_FromPyObjectNew(value);
175176
}
176177
return 0;
177178
}

Objects/genobject.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ _PyGen_yf(PyGenObject *gen)
345345
_PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
346346
assert(is_resume(frame->instr_ptr));
347347
assert((frame->instr_ptr->op.arg & RESUME_OPARG_LOCATION_MASK) >= RESUME_AFTER_YIELD_FROM);
348-
return PyStackRef_AsPyObjectBorrow(PyStackRef_DUP(_PyFrame_StackPeek(frame)));
348+
return PyStackRef_AsPyObjectNew(_PyFrame_StackPeek(frame));
349349
}
350350
return NULL;
351351
}

Programs/test_frozenmain.h

Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/bytecodes.c

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,7 +1470,7 @@ dummy_func(
14701470

14711471
inst(STORE_GLOBAL, (v --)) {
14721472
PyObject *name = GETITEM(FRAME_CO_NAMES, oparg);
1473-
int err = PyDict_SetItem(GLOBALS(), name, PyStackRef_AsPyObjectSteal(v));
1473+
int err = PyDict_SetItem(GLOBALS(), name, PyStackRef_AsPyObjectBorrow(v));
14741474
DECREF_INPUTS();
14751475
ERROR_IF(err, error);
14761476
}
@@ -1490,13 +1490,13 @@ dummy_func(
14901490
}
14911491

14921492
inst(LOAD_LOCALS, ( -- locals)) {
1493-
_PyStackRef locals_s = PyStackRef_FromPyObjectSteal(LOCALS());
1494-
if (PyStackRef_IsNull(locals_s)) {
1493+
PyObject *l = LOCALS();
1494+
if (l == NULL) {
14951495
_PyErr_SetString(tstate, PyExc_SystemError,
14961496
"no locals found");
14971497
ERROR_IF(true, error);
14981498
}
1499-
locals = PyStackRef_DUP(locals_s);
1499+
locals = PyStackRef_FromPyObjectNew(l);;
15001500
}
15011501

15021502
inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v)) {
@@ -2479,7 +2479,7 @@ dummy_func(
24792479
int sign_ish = COMPARISON_BIT(dleft, dright);
24802480
_Py_DECREF_SPECIALIZED(left_o, _PyFloat_ExactDealloc);
24812481
_Py_DECREF_SPECIALIZED(right_o, _PyFloat_ExactDealloc);
2482-
res = PyStackRef_FromPyObjectSteal((sign_ish & oparg) ? Py_True : Py_False);
2482+
res = (sign_ish & oparg) ? PyStackRef_True() : PyStackRef_False();
24832483
// It's always a bool, so we don't care about oparg & 16.
24842484
}
24852485

@@ -2499,7 +2499,7 @@ dummy_func(
24992499
int sign_ish = COMPARISON_BIT(ileft, iright);
25002500
_Py_DECREF_SPECIALIZED(left_o, (destructor)PyObject_Free);
25012501
_Py_DECREF_SPECIALIZED(right_o, (destructor)PyObject_Free);
2502-
res = PyStackRef_FromPyObjectSteal((sign_ish & oparg) ? Py_True : Py_False);
2502+
res = (sign_ish & oparg) ? PyStackRef_True() : PyStackRef_False();
25032503
// It's always a bool, so we don't care about oparg & 16.
25042504
}
25052505

@@ -2516,7 +2516,7 @@ dummy_func(
25162516
assert(eq == 0 || eq == 1);
25172517
assert((oparg & 0xf) == COMPARISON_NOT_EQUALS || (oparg & 0xf) == COMPARISON_EQUALS);
25182518
assert(COMPARISON_NOT_EQUALS + 1 == COMPARISON_EQUALS);
2519-
res = PyStackRef_FromPyObjectSteal(((COMPARISON_NOT_EQUALS + eq) & oparg) ? Py_True : Py_False);
2519+
res = ((COMPARISON_NOT_EQUALS + eq) & oparg) ? PyStackRef_True() : PyStackRef_False();
25202520
// It's always a bool, so we don't care about oparg & 16.
25212521
}
25222522

@@ -2526,7 +2526,7 @@ dummy_func(
25262526

25272527
int res = Py_Is(left_o, right_o) ^ oparg;
25282528
DECREF_INPUTS();
2529-
b = PyStackRef_FromPyObjectSteal(res ? Py_True : Py_False);
2529+
b = res ? PyStackRef_True() : PyStackRef_False();
25302530
}
25312531

25322532
family(CONTAINS_OP, INLINE_CACHE_ENTRIES_CONTAINS_OP) = {
@@ -2541,7 +2541,7 @@ dummy_func(
25412541
int res = PySequence_Contains(right_o, left_o);
25422542
DECREF_INPUTS();
25432543
ERROR_IF(res < 0, error);
2544-
b = PyStackRef_FromPyObjectSteal((res ^ oparg) ? Py_True : Py_False);
2544+
b = (res ^ oparg) ? PyStackRef_True() : PyStackRef_False();
25452545
}
25462546

25472547
specializing op(_SPECIALIZE_CONTAINS_OP, (counter/1, left, right -- left, right)) {
@@ -2568,7 +2568,7 @@ dummy_func(
25682568
int res = _PySet_Contains((PySetObject *)right_o, left_o);
25692569
DECREF_INPUTS();
25702570
ERROR_IF(res < 0, error);
2571-
b = PyStackRef_FromPyObjectSteal((res ^ oparg) ? Py_True : Py_False);
2571+
b = (res ^ oparg) ? PyStackRef_True() : PyStackRef_False();
25722572
}
25732573

25742574
inst(CONTAINS_OP_DICT, (unused/1, left, right -- b)) {
@@ -2580,7 +2580,7 @@ dummy_func(
25802580
int res = PyDict_Contains(right_o, left_o);
25812581
DECREF_INPUTS();
25822582
ERROR_IF(res < 0, error);
2583-
b = PyStackRef_FromPyObjectSteal((res ^ oparg) ? Py_True : Py_False);
2583+
b = (res ^ oparg) ? PyStackRef_True() : PyStackRef_False();
25842584
}
25852585

25862586
inst(CHECK_EG_MATCH, (exc_value_st, match_type_st -- rest, match)) {
@@ -2621,7 +2621,7 @@ dummy_func(
26212621

26222622
int res = PyErr_GivenExceptionMatches(left_o, right_o);
26232623
DECREF_INPUTS();
2624-
b = PyStackRef_FromPyObjectSteal(res ? Py_True : Py_False);
2624+
b = res ? PyStackRef_True() : PyStackRef_False();
26252625
}
26262626

26272627
tier1 inst(IMPORT_NAME, (level, fromlist -- res)) {
@@ -3503,17 +3503,17 @@ dummy_func(
35033503
assert(Py_TYPE(callable_o) == &PyFunction_Type);
35043504
int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags;
35053505
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o));
3506-
_PyInterpreterFrame *new_frame_o = _PyEvalFramePushAndInit(
3506+
_PyInterpreterFrame *_new_frame = _PyEvalFramePushAndInit(
35073507
tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals,
35083508
args, total_args, NULL
35093509
);
35103510
// The frame has stolen all the arguments from the stack,
35113511
// so there is no need to clean them up.
35123512
SYNC_SP();
3513-
if (new_frame_o == NULL) {
3513+
if (_new_frame == NULL) {
35143514
ERROR_NO_POP();
35153515
}
3516-
new_frame = (_PyStackRef) { .bits = (uintptr_t)new_frame_o };
3516+
new_frame = (_PyStackRef) { .bits = (uintptr_t)_new_frame };
35173517
}
35183518

35193519
op(_CHECK_FUNCTION_VERSION, (func_version/2, callable, unused, unused[oparg] -- callable, unused, unused[oparg])) {
@@ -3647,13 +3647,13 @@ dummy_func(
36473647
int has_self = !PyStackRef_IsNull(self_or_null);
36483648
STAT_INC(CALL, hit);
36493649
PyFunctionObject *func = (PyFunctionObject *)callable_o;
3650-
_PyInterpreterFrame *new_frame_o = _PyFrame_PushUnchecked(tstate, func, oparg + has_self);
3651-
_PyStackRef *first_non_self_local = new_frame_o->localsplus + has_self;
3652-
new_frame_o->localsplus[0] = self_or_null;
3650+
_PyInterpreterFrame *_new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self);
3651+
_PyStackRef *first_non_self_local = _new_frame->localsplus + has_self;
3652+
_new_frame->localsplus[0] = self_or_null;
36533653
for (int i = 0; i < oparg; i++) {
36543654
first_non_self_local[i] = args[i];
36553655
}
3656-
new_frame = (_PyStackRef) { .bits = (uintptr_t)new_frame_o };
3656+
new_frame = (_PyStackRef) { .bits = (uintptr_t)_new_frame };
36573657
}
36583658

36593659
op(_PUSH_FRAME, (new_frame -- )) {
@@ -3938,7 +3938,6 @@ dummy_func(
39383938
DECREF_INPUTS();
39393939
ERROR_IF(true, error);
39403940
}
3941-
ERROR_IF(args_o == NULL, error);
39423941
PyObject *res_o = cfunc(PyCFunction_GET_SELF(callable_o), args_o, total_args, NULL);
39433942
STACKREFS_TO_PYOBJECTS_CLEANUP(args_o);
39443943

Python/ceval_macros.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,4 +470,4 @@ do { \
470470
#else
471471
#define STACKREFS_TO_PYOBJECTS_CLEANUP(NAME) \
472472
(void)(NAME);
473-
#endif
473+
#endif

Python/executor_cases.c.h

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

0 commit comments

Comments
 (0)
0