Skip to content

gh-106307: Add PyMapping_GetOptionalItem() #106308

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Jul 11, 2023
Merged

gh-106307: Add PyMapping_GetOptionalItem() #106308

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
078ea48
gh-106307: Add _PyMapping_LookupItem()
serhiy-storchaka Jul 1, 2023
00a3743
Make them public C API.
serhiy-storchaka Jul 7, 2023
4b3fc37
Apply suggestions from code review
serhiy-storchaka Jul 8, 2023
a176323
Apply suggestions from code review
serhiy-storchaka Jul 8, 2023
0f6cb97
Cleanup.
serhiy-storchaka Jul 8, 2023
0c2244a
Add a news entry.
serhiy-storchaka Jul 8, 2023
576bcf5
Move new entries to the end of Misc/stable_abi.toml.
serhiy-storchaka Jul 10, 2023
a9f41bd
Merge branch 'main' into _PyMapping_LookupItem
serhiy-storchaka Jul 10, 2023
d6a9204
Rewrite documentation according to review in #106522.
serhiy-storchaka Jul 11, 2023
07a169f
Merge branch 'main' into _PyMapping_LookupItem
serhiy-storchaka Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Include/cpython/object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,17 @@ PyAPI_FUNC(int)
_PyObject_GenericSetAttrWithDict(PyObject *, PyObject *,
PyObject *, PyObject *);

/* Replacements of PyObject_GetItem() which don't raise KeyError.

Return 1 and set *result != NULL if a key is found.
Return 0 and set *result == NULL if a key is not found;
a KeyError is silenced.
Return -1 and set *result == NULL if an error other than KeyError
is raised.
*/
PyAPI_FUNC(int) _PyMapping_LookupItem(PyObject *, PyObject *, PyObject **);
PyAPI_FUNC(int) _PyMapping_LookupItemString(PyObject *, const char *, PyObject **);

PyAPI_FUNC(PyObject *) _PyObject_FunctionStr(PyObject *);

/* Safely decref `dst` and set `dst` to `src`.
Expand Down
22 changes: 9 additions & 13 deletions Modules/_lzmamodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,15 +242,10 @@ parse_filter_spec_lzma(_lzma_state *state, PyObject *spec)
/* First, fill in default values for all the options using a preset.
Then, override the defaults with any values given by the caller. */

preset_obj = PyMapping_GetItemString(spec, "preset");
if (preset_obj == NULL) {
if (PyErr_ExceptionMatches(PyExc_KeyError)) {
PyErr_Clear();
}
else {
return NULL;
}
} else {
if (_PyMapping_LookupItemString(spec, "preset", &preset_obj) < 0) {
return NULL;
}
if (preset_obj != NULL) {
int ok = uint32_converter(preset_obj, &preset);
Py_DECREF(preset_obj);
if (!ok) {
Expand Down Expand Up @@ -347,11 +342,12 @@ lzma_filter_converter(_lzma_state *state, PyObject *spec, void *ptr)
"Filter specifier must be a dict or dict-like object");
return 0;
}
id_obj = PyMapping_GetItemString(spec, "id");
if (_PyMapping_LookupItemString(spec, "id", &id_obj) < 0) {
return 0;
}
if (id_obj == NULL) {
if (PyErr_ExceptionMatches(PyExc_KeyError))
PyErr_SetString(PyExc_ValueError,
"Filter specifier must have an \"id\" entry");
PyErr_SetString(PyExc_ValueError,
"Filter specifier must have an \"id\" entry");
return 0;
}
f->id = PyLong_AsUnsignedLongLong(id_obj);
Expand Down
10 changes: 3 additions & 7 deletions Modules/_pickle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4438,13 +4438,9 @@ save(PickleState *st, PicklerObject *self, PyObject *obj, int pers_save)
Py_INCREF(reduce_func);
}
} else {
reduce_func = PyObject_GetItem(self->dispatch_table,
(PyObject *)type);
if (reduce_func == NULL) {
if (PyErr_ExceptionMatches(PyExc_KeyError))
PyErr_Clear();
else
goto error;
if (_PyMapping_LookupItem(self->dispatch_table, (PyObject *)type,
&reduce_func) < 0) {
goto error;
}
}
if (reduce_func != NULL) {
Expand Down
40 changes: 40 additions & 0 deletions Objects/abstract.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,30 @@ PyObject_GetItem(PyObject *o, PyObject *key)
return type_error("'%.200s' object is not subscriptable", o);
}

int
_PyMapping_LookupItem(PyObject *o, PyObject *key, PyObject **pvalue)
{
if (PyDict_CheckExact(o)) {
*pvalue = PyDict_GetItemWithError(o, key); /* borrowed */
if (*pvalue) {
Py_INCREF(*pvalue);
return 1;
}
return PyErr_Occurred() ? -1 : 0;
}

*pvalue = PyObject_GetItem(o, key);
if (*pvalue) {
return 1;
}
assert(PyErr_Occurred());
if (!PyErr_ExceptionMatches(PyExc_KeyError)) {
return -1;
}
PyErr_Clear();
return 0;
}

int
PyObject_SetItem(PyObject *o, PyObject *key, PyObject *value)
{
Expand Down Expand Up @@ -2366,6 +2390,22 @@ PyMapping_GetItemString(PyObject *o, const char *key)
return r;
}

int
_PyMapping_LookupItemString(PyObject *o, const char *key, PyObject **pvalue)
{
if (key == NULL) {
null_error();
return -1;
}
PyObject *okey = PyUnicode_FromString(key);
if (okey == NULL) {
return -1;
}
int r = _PyMapping_LookupItem(o, okey, pvalue);
Py_DECREF(okey);
return r;
}

int
PyMapping_SetItemString(PyObject *o, const char *key, PyObject *value)
{
Expand Down
116 changes: 24 additions & 92 deletions Python/bytecodes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1086,26 +1086,11 @@ dummy_func(
}

inst(LOAD_BUILD_CLASS, ( -- bc)) {
if (PyDict_CheckExact(BUILTINS())) {
bc = _PyDict_GetItemWithError(BUILTINS(),
&_Py_ID(__build_class__));
if (bc == NULL) {
if (!_PyErr_Occurred(tstate)) {
_PyErr_SetString(tstate, PyExc_NameError,
"__build_class__ not found");
}
ERROR_IF(true, error);
}
Py_INCREF(bc);
}
else {
bc = PyObject_GetItem(BUILTINS(), &_Py_ID(__build_class__));
if (bc == NULL) {
if (_PyErr_ExceptionMatches(tstate, PyExc_KeyError))
_PyErr_SetString(tstate, PyExc_NameError,
"__build_class__ not found");
ERROR_IF(true, error);
}
ERROR_IF(_PyMapping_LookupItem(BUILTINS(), &_Py_ID(__build_class__), &bc) < 0, error);
if (bc == NULL) {
_PyErr_SetString(tstate, PyExc_NameError,
"__build_class__ not found");
ERROR_IF(true, error);
}
}

Expand Down Expand Up @@ -1280,25 +1265,9 @@ dummy_func(

op(_LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v)) {
PyObject *name = GETITEM(FRAME_CO_NAMES, oparg);
if (PyDict_CheckExact(mod_or_class_dict)) {
v = PyDict_GetItemWithError(mod_or_class_dict, name);
if (v != NULL) {
Py_INCREF(v);
}
else if (_PyErr_Occurred(tstate)) {
Py_DECREF(mod_or_class_dict);
goto error;
}
}
else {
v = PyObject_GetItem(mod_or_class_dict, name);
if (v == NULL) {
if (!_PyErr_ExceptionMatches(tstate, PyExc_KeyError)) {
Py_DECREF(mod_or_class_dict);
goto error;
}
_PyErr_Clear(tstate);
}
if (_PyMapping_LookupItem(mod_or_class_dict, name, &v) < 0) {
Py_DECREF(mod_or_class_dict);
goto error;
}
Py_DECREF(mod_or_class_dict);
if (v == NULL) {
Expand All @@ -1310,28 +1279,14 @@ dummy_func(
goto error;
}
else {
if (PyDict_CheckExact(BUILTINS())) {
v = PyDict_GetItemWithError(BUILTINS(), name);
if (v == NULL) {
if (!_PyErr_Occurred(tstate)) {
format_exc_check_arg(
tstate, PyExc_NameError,
NAME_ERROR_MSG, name);
}
goto error;
}
Py_INCREF(v);
if (_PyMapping_LookupItem(BUILTINS(), name, &v) < 0) {
goto error;
}
else {
v = PyObject_GetItem(BUILTINS(), name);
if (v == NULL) {
if (_PyErr_ExceptionMatches(tstate, PyExc_KeyError)) {
format_exc_check_arg(
tstate, PyExc_NameError,
NAME_ERROR_MSG, name);
}
goto error;
}
if (v == NULL) {
format_exc_check_arg(
tstate, PyExc_NameError,
NAME_ERROR_MSG, name);
goto error;
}
}
}
Expand Down Expand Up @@ -1381,19 +1336,14 @@ dummy_func(
/* Slow-path if globals or builtins is not a dict */

/* namespace 1: globals */
v = PyObject_GetItem(GLOBALS(), name);
ERROR_IF(_PyMapping_LookupItem(GLOBALS(), name, &v) < 0, error);
if (v == NULL) {
ERROR_IF(!_PyErr_ExceptionMatches(tstate, PyExc_KeyError), error);
_PyErr_Clear(tstate);

/* namespace 2: builtins */
v = PyObject_GetItem(BUILTINS(), name);
ERROR_IF(_PyMapping_LookupItem(BUILTINS(), name, &v) < 0, error);
if (v == NULL) {
if (_PyErr_ExceptionMatches(tstate, PyExc_KeyError)) {
format_exc_check_arg(
tstate, PyExc_NameError,
NAME_ERROR_MSG, name);
}
format_exc_check_arg(
tstate, PyExc_NameError,
NAME_ERROR_MSG, name);
ERROR_IF(true, error);
}
}
Expand Down Expand Up @@ -1466,25 +1416,9 @@ dummy_func(
assert(class_dict);
assert(oparg >= 0 && oparg < _PyFrame_GetCode(frame)->co_nlocalsplus);
name = PyTuple_GET_ITEM(_PyFrame_GetCode(frame)->co_localsplusnames, oparg);
if (PyDict_CheckExact(class_dict)) {
value = PyDict_GetItemWithError(class_dict, name);
if (value != NULL) {
Py_INCREF(value);
}
else if (_PyErr_Occurred(tstate)) {
Py_DECREF(class_dict);
goto error;
}
}
else {
value = PyObject_GetItem(class_dict, name);
if (value == NULL) {
if (!_PyErr_ExceptionMatches(tstate, PyExc_KeyError)) {
Py_DECREF(class_dict);
goto error;
}
_PyErr_Clear(tstate);
}
if (_PyMapping_LookupItem(class_dict, name, &value) < 0) {
Py_DECREF(class_dict);
goto error;
}
Py_DECREF(class_dict);
if (!value) {
Expand Down Expand Up @@ -1622,10 +1556,8 @@ dummy_func(
}
else {
/* do the same if locals() is not a dict */
ann_dict = PyObject_GetItem(LOCALS(), &_Py_ID(__annotations__));
ERROR_IF(_PyMapping_LookupItem(LOCALS(), &_Py_ID(__annotations__), &ann_dict) < 0, error);
if (ann_dict == NULL) {
ERROR_IF(!_PyErr_ExceptionMatches(tstate, PyExc_KeyError), error);
_PyErr_Clear(tstate);
ann_dict = PyDict_New();
ERROR_IF(ann_dict == NULL, error);
err = PyObject_SetItem(LOCALS(), &_Py_ID(__annotations__),
Expand Down
Loading