gh-108308: Remove _PyDict_GetItemStringWithError() function (#108426)

Remove the internal _PyDict_GetItemStringWithError() function. It can
now be replaced with the new public PyDict_ContainsString() and
PyDict_GetItemStringRef() functions.

getargs.c now now uses a strong reference for current_arg.
find_keyword() returns a strong reference.
This commit is contained in:
Victor Stinner 2023-08-24 17:34:22 +02:00 committed by GitHub
parent ea871c9b0f
commit 52c6a6e48a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 36 additions and 44 deletions

View File

@ -13,7 +13,6 @@ extern "C" {
// Unsafe flavor of PyDict_GetItemWithError(): no error checking // Unsafe flavor of PyDict_GetItemWithError(): no error checking
extern PyObject* _PyDict_GetItemWithError(PyObject *dp, PyObject *key); extern PyObject* _PyDict_GetItemWithError(PyObject *dp, PyObject *key);
extern PyObject* _PyDict_GetItemStringWithError(PyObject *, const char *);
extern int _PyDict_Contains_KnownHash(PyObject *, PyObject *, Py_hash_t); extern int _PyDict_Contains_KnownHash(PyObject *, PyObject *, Py_hash_t);

View File

@ -1829,19 +1829,6 @@ _PyDict_GetItemIdWithError(PyObject *dp, _Py_Identifier *key)
return _PyDict_GetItem_KnownHash(dp, kv, hash); // borrowed reference return _PyDict_GetItem_KnownHash(dp, kv, hash); // borrowed reference
} }
PyObject *
_PyDict_GetItemStringWithError(PyObject *v, const char *key)
{
PyObject *kv, *rv;
kv = PyUnicode_FromString(key);
if (kv == NULL) {
return NULL;
}
rv = PyDict_GetItemWithError(v, kv);
Py_DECREF(kv);
return rv; // borrowed reference
}
/* Fast version of global value lookup (LOAD_GLOBAL). /* Fast version of global value lookup (LOAD_GLOBAL).
* Lookup in globals, then builtins. * Lookup in globals, then builtins.
* *

View File

@ -1491,7 +1491,6 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
int i, pos, len; int i, pos, len;
int skip = 0; int skip = 0;
Py_ssize_t nargs, nkwargs; Py_ssize_t nargs, nkwargs;
PyObject *current_arg;
freelistentry_t static_entries[STATIC_FREELIST_ENTRIES]; freelistentry_t static_entries[STATIC_FREELIST_ENTRIES];
freelist_t freelist; freelist_t freelist;
@ -1621,17 +1620,17 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
return cleanreturn(0, &freelist); return cleanreturn(0, &freelist);
} }
if (!skip) { if (!skip) {
PyObject *current_arg;
if (i < nargs) { if (i < nargs) {
current_arg = PyTuple_GET_ITEM(args, i); current_arg = Py_NewRef(PyTuple_GET_ITEM(args, i));
} }
else if (nkwargs && i >= pos) { else if (nkwargs && i >= pos) {
current_arg = _PyDict_GetItemStringWithError(kwargs, kwlist[i]); if (PyDict_GetItemStringRef(kwargs, kwlist[i], &current_arg) < 0) {
return cleanreturn(0, &freelist);
}
if (current_arg) { if (current_arg) {
--nkwargs; --nkwargs;
} }
else if (PyErr_Occurred()) {
return cleanreturn(0, &freelist);
}
} }
else { else {
current_arg = NULL; current_arg = NULL;
@ -1640,6 +1639,7 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
if (current_arg) { if (current_arg) {
msg = convertitem(current_arg, &format, p_va, flags, msg = convertitem(current_arg, &format, p_va, flags,
levels, msgbuf, sizeof(msgbuf), &freelist); levels, msgbuf, sizeof(msgbuf), &freelist);
Py_DECREF(current_arg);
if (msg) { if (msg) {
seterror(i+1, msg, levels, fname, custom_msg); seterror(i+1, msg, levels, fname, custom_msg);
return cleanreturn(0, &freelist); return cleanreturn(0, &freelist);
@ -1710,8 +1710,12 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
Py_ssize_t j; Py_ssize_t j;
/* make sure there are no arguments given by name and position */ /* make sure there are no arguments given by name and position */
for (i = pos; i < nargs; i++) { for (i = pos; i < nargs; i++) {
current_arg = _PyDict_GetItemStringWithError(kwargs, kwlist[i]); PyObject *current_arg;
if (PyDict_GetItemStringRef(kwargs, kwlist[i], &current_arg) < 0) {
return cleanreturn(0, &freelist);
}
if (current_arg) { if (current_arg) {
Py_DECREF(current_arg);
/* arg present in tuple and in dict */ /* arg present in tuple and in dict */
PyErr_Format(PyExc_TypeError, PyErr_Format(PyExc_TypeError,
"argument for %.200s%s given by name ('%s') " "argument for %.200s%s given by name ('%s') "
@ -1721,9 +1725,6 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
kwlist[i], i+1); kwlist[i], i+1);
return cleanreturn(0, &freelist); return cleanreturn(0, &freelist);
} }
else if (PyErr_Occurred()) {
return cleanreturn(0, &freelist);
}
} }
/* make sure there are no extraneous keyword arguments */ /* make sure there are no extraneous keyword arguments */
j = 0; j = 0;
@ -1985,7 +1986,7 @@ find_keyword(PyObject *kwnames, PyObject *const *kwstack, PyObject *key)
/* kwname == key will normally find a match in since keyword keys /* kwname == key will normally find a match in since keyword keys
should be interned strings; if not retry below in a new loop. */ should be interned strings; if not retry below in a new loop. */
if (kwname == key) { if (kwname == key) {
return kwstack[i]; return Py_NewRef(kwstack[i]);
} }
} }
@ -1993,7 +1994,7 @@ find_keyword(PyObject *kwnames, PyObject *const *kwstack, PyObject *key)
PyObject *kwname = PyTuple_GET_ITEM(kwnames, i); PyObject *kwname = PyTuple_GET_ITEM(kwnames, i);
assert(PyUnicode_Check(kwname)); assert(PyUnicode_Check(kwname));
if (_PyUnicode_EQ(kwname, key)) { if (_PyUnicode_EQ(kwname, key)) {
return kwstack[i]; return Py_NewRef(kwstack[i]);
} }
} }
return NULL; return NULL;
@ -2013,7 +2014,6 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
PyObject *keyword; PyObject *keyword;
int i, pos, len; int i, pos, len;
Py_ssize_t nkwargs; Py_ssize_t nkwargs;
PyObject *current_arg;
freelistentry_t static_entries[STATIC_FREELIST_ENTRIES]; freelistentry_t static_entries[STATIC_FREELIST_ENTRIES];
freelist_t freelist; freelist_t freelist;
PyObject *const *kwstack = NULL; PyObject *const *kwstack = NULL;
@ -2108,14 +2108,14 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
} }
assert(!IS_END_OF_FORMAT(*format)); assert(!IS_END_OF_FORMAT(*format));
PyObject *current_arg;
if (i < nargs) { if (i < nargs) {
current_arg = args[i]; current_arg = Py_NewRef(args[i]);
} }
else if (nkwargs && i >= pos) { else if (nkwargs && i >= pos) {
keyword = PyTuple_GET_ITEM(kwtuple, i - pos); keyword = PyTuple_GET_ITEM(kwtuple, i - pos);
if (kwargs != NULL) { if (kwargs != NULL) {
current_arg = PyDict_GetItemWithError(kwargs, keyword); if (PyDict_GetItemRef(kwargs, keyword, &current_arg) < 0) {
if (!current_arg && PyErr_Occurred()) {
return cleanreturn(0, &freelist); return cleanreturn(0, &freelist);
} }
} }
@ -2133,6 +2133,7 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
if (current_arg) { if (current_arg) {
msg = convertitem(current_arg, &format, p_va, flags, msg = convertitem(current_arg, &format, p_va, flags,
levels, msgbuf, sizeof(msgbuf), &freelist); levels, msgbuf, sizeof(msgbuf), &freelist);
Py_DECREF(current_arg);
if (msg) { if (msg) {
seterror(i+1, msg, levels, parser->fname, parser->custom_msg); seterror(i+1, msg, levels, parser->fname, parser->custom_msg);
return cleanreturn(0, &freelist); return cleanreturn(0, &freelist);
@ -2183,10 +2184,10 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
if (nkwargs > 0) { if (nkwargs > 0) {
/* make sure there are no arguments given by name and position */ /* make sure there are no arguments given by name and position */
for (i = pos; i < nargs; i++) { for (i = pos; i < nargs; i++) {
PyObject *current_arg;
keyword = PyTuple_GET_ITEM(kwtuple, i - pos); keyword = PyTuple_GET_ITEM(kwtuple, i - pos);
if (kwargs != NULL) { if (kwargs != NULL) {
current_arg = PyDict_GetItemWithError(kwargs, keyword); if (PyDict_GetItemRef(kwargs, keyword, &current_arg) < 0) {
if (!current_arg && PyErr_Occurred()) {
return cleanreturn(0, &freelist); return cleanreturn(0, &freelist);
} }
} }
@ -2194,6 +2195,7 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
current_arg = find_keyword(kwnames, kwstack, keyword); current_arg = find_keyword(kwnames, kwstack, keyword);
} }
if (current_arg) { if (current_arg) {
Py_DECREF(current_arg);
/* arg present in tuple and in dict */ /* arg present in tuple and in dict */
PyErr_Format(PyExc_TypeError, PyErr_Format(PyExc_TypeError,
"argument for %.200s%s given by name ('%U') " "argument for %.200s%s given by name ('%U') "
@ -2248,7 +2250,6 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
int i, posonly, minposonly, maxargs; int i, posonly, minposonly, maxargs;
int reqlimit = minkw ? maxpos + minkw : minpos; int reqlimit = minkw ? maxpos + minkw : minpos;
Py_ssize_t nkwargs; Py_ssize_t nkwargs;
PyObject *current_arg;
PyObject * const *kwstack = NULL; PyObject * const *kwstack = NULL;
assert(kwargs == NULL || PyDict_Check(kwargs)); assert(kwargs == NULL || PyDict_Check(kwargs));
@ -2343,11 +2344,11 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
/* copy keyword args using kwtuple to drive process */ /* copy keyword args using kwtuple to drive process */
for (i = Py_MAX((int)nargs, posonly); i < maxargs; i++) { for (i = Py_MAX((int)nargs, posonly); i < maxargs; i++) {
PyObject *current_arg;
if (nkwargs) { if (nkwargs) {
keyword = PyTuple_GET_ITEM(kwtuple, i - posonly); keyword = PyTuple_GET_ITEM(kwtuple, i - posonly);
if (kwargs != NULL) { if (kwargs != NULL) {
current_arg = PyDict_GetItemWithError(kwargs, keyword); if (PyDict_GetItemRef(kwargs, keyword, &current_arg) < 0) {
if (!current_arg && PyErr_Occurred()) {
return NULL; return NULL;
} }
} }
@ -2365,6 +2366,7 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
buf[i] = current_arg; buf[i] = current_arg;
if (current_arg) { if (current_arg) {
Py_DECREF(current_arg);
--nkwargs; --nkwargs;
} }
else if (i < minpos || (maxpos <= i && i < reqlimit)) { else if (i < minpos || (maxpos <= i && i < reqlimit)) {
@ -2382,10 +2384,10 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
if (nkwargs > 0) { if (nkwargs > 0) {
/* make sure there are no arguments given by name and position */ /* make sure there are no arguments given by name and position */
for (i = posonly; i < nargs; i++) { for (i = posonly; i < nargs; i++) {
PyObject *current_arg;
keyword = PyTuple_GET_ITEM(kwtuple, i - posonly); keyword = PyTuple_GET_ITEM(kwtuple, i - posonly);
if (kwargs != NULL) { if (kwargs != NULL) {
current_arg = PyDict_GetItemWithError(kwargs, keyword); if (PyDict_GetItemRef(kwargs, keyword, &current_arg) < 0) {
if (!current_arg && PyErr_Occurred()) {
return NULL; return NULL;
} }
} }
@ -2393,6 +2395,7 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
current_arg = find_keyword(kwnames, kwstack, keyword); current_arg = find_keyword(kwnames, kwstack, keyword);
} }
if (current_arg) { if (current_arg) {
Py_DECREF(current_arg);
/* arg present in tuple and in dict */ /* arg present in tuple and in dict */
PyErr_Format(PyExc_TypeError, PyErr_Format(PyExc_TypeError,
"argument for %.200s%s given by name ('%U') " "argument for %.200s%s given by name ('%U') "
@ -2424,7 +2427,6 @@ _PyArg_UnpackKeywordsWithVararg(PyObject *const *args, Py_ssize_t nargs,
int i, posonly, minposonly, maxargs; int i, posonly, minposonly, maxargs;
int reqlimit = minkw ? maxpos + minkw : minpos; int reqlimit = minkw ? maxpos + minkw : minpos;
Py_ssize_t nkwargs; Py_ssize_t nkwargs;
PyObject *current_arg;
PyObject * const *kwstack = NULL; PyObject * const *kwstack = NULL;
assert(kwargs == NULL || PyDict_Check(kwargs)); assert(kwargs == NULL || PyDict_Check(kwargs));
@ -2497,13 +2499,12 @@ _PyArg_UnpackKeywordsWithVararg(PyObject *const *args, Py_ssize_t nargs,
} }
/* copy keyword args using kwtuple to drive process */ /* copy keyword args using kwtuple to drive process */
for (i = Py_MAX((int)nargs, posonly) - for (i = Py_MAX((int)nargs, posonly) - Py_SAFE_DOWNCAST(varargssize, Py_ssize_t, int); i < maxargs; i++) {
Py_SAFE_DOWNCAST(varargssize, Py_ssize_t, int); i < maxargs; i++) { PyObject *current_arg;
if (nkwargs) { if (nkwargs) {
keyword = PyTuple_GET_ITEM(kwtuple, i - posonly); keyword = PyTuple_GET_ITEM(kwtuple, i - posonly);
if (kwargs != NULL) { if (kwargs != NULL) {
current_arg = PyDict_GetItemWithError(kwargs, keyword); if (PyDict_GetItemRef(kwargs, keyword, &current_arg) < 0) {
if (!current_arg && PyErr_Occurred()) {
goto exit; goto exit;
} }
} }
@ -2536,6 +2537,7 @@ _PyArg_UnpackKeywordsWithVararg(PyObject *const *args, Py_ssize_t nargs,
} }
if (current_arg) { if (current_arg) {
Py_DECREF(current_arg);
--nkwargs; --nkwargs;
} }
else if (i < minpos || (maxpos <= i && i < reqlimit)) { else if (i < minpos || (maxpos <= i && i < reqlimit)) {

View File

@ -15,7 +15,6 @@
#include "pycore_ast.h" // PyAST_mod2obj #include "pycore_ast.h" // PyAST_mod2obj
#include "pycore_ceval.h" // _Py_EnterRecursiveCall #include "pycore_ceval.h" // _Py_EnterRecursiveCall
#include "pycore_compile.h" // _PyAST_Compile() #include "pycore_compile.h" // _PyAST_Compile()
#include "pycore_dict.h" // _PyDict_GetItemStringWithError()
#include "pycore_interp.h" // PyInterpreterState.importlib #include "pycore_interp.h" // PyInterpreterState.importlib
#include "pycore_object.h" // _PyDebug_PrintTotalRefs() #include "pycore_object.h" // _PyDebug_PrintTotalRefs()
#include "pycore_parser.h" // _PyParser_ASTFromString() #include "pycore_parser.h" // _PyParser_ASTFromString()

View File

@ -87,7 +87,12 @@ _PySys_GetObject(PyInterpreterState *interp, const char *name)
if (sysdict == NULL) { if (sysdict == NULL) {
return NULL; return NULL;
} }
return _PyDict_GetItemStringWithError(sysdict, name); PyObject *value;
if (PyDict_GetItemStringRef(sysdict, name, &value) != 1) {
return NULL;
}
Py_DECREF(value); // return a borrowed reference
return value;
} }
PyObject * PyObject *