From 999039361c58b47c0a1311a603ddf5585b7b53ba Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 1 May 2021 17:20:40 -0700 Subject: [PATCH 1/8] bpo-44003: Add a failing unittest for the issue. --- Lib/test/test_functools.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 3320ab7ec6649d..68320c306a1df9 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -1728,6 +1728,16 @@ def test_staticmethod(x): for ref in refs: self.assertIsNone(ref()) + def test_lru_defaults_bug44003(self): + @self.module.lru_cache(maxsize=None) + def func(arg='ARG', *, kw='KW'): + return arg, kw + + self.assertEqual(func.__wrapped__.__defaults__, ('ARG',)) + self.assertEqual(func.__wrapped__.__kwdefaults__, {'kw': 'KW'}) + self.assertEqual(func.__defaults__, ('ARG',)) + self.assertEqual(func.__kwdefaults__, {'kw': 'KW'}) + @py_functools.lru_cache() def py_cached_func(x, y): From 8074cb2abb08bc421965df0c6a399cef059b06aa Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 1 May 2021 17:21:30 -0700 Subject: [PATCH 2/8] Fix bpo-44003 for the C _functools module. --- Modules/_functoolsmodule.c | 64 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 19cfa9b07b340d..81e779b8392501 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -19,6 +19,7 @@ typedef struct _functools_state { PyTypeObject *partial_type; PyTypeObject *keyobject_type; PyTypeObject *lru_list_elem_type; + PyObject *list_of_lru_obj_attrs_to_clone; } _functools_state; static inline _functools_state * @@ -1138,7 +1139,8 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds static PyObject * lru_cache_new(PyTypeObject *type, PyObject *args, PyObject *kw) { - PyObject *func, *maxsize_O, *cache_info_type, *cachedict; + PyObject *func, *maxsize_O, *cache_info_type, *cachedict, *obj_dict; + PyObject *defaults_str; int typed; lru_cache_object *obj; Py_ssize_t maxsize; @@ -1188,9 +1190,43 @@ lru_cache_new(PyTypeObject *type, PyObject *args, PyObject *kw) if (!(cachedict = PyDict_New())) return NULL; + obj_dict = PyDict_New(); + if (obj_dict == NULL) { + Py_DECREF(cachedict); + return NULL; + } + + /* Copy special attributes from the original function over to ours. */ + { + Py_ssize_t n_attrs = PyTuple_GET_SIZE(state->list_of_lru_obj_attrs_to_clone); + for (Py_ssize_t idx = 0; idx < n_attrs; ++idx) { + PyObject *attr_name = PyTuple_GET_ITEM(state->list_of_lru_obj_attrs_to_clone, idx); + if (attr_name == NULL) { + Py_DECREF(cachedict); + Py_DECREF(obj_dict); + return NULL; + } + PyObject *attr = PyObject_GetAttr(func, attr_name); + if (attr != NULL) { + int err = PyDict_SetItem(obj_dict, attr_name, attr); + if (err != 0) { + Py_DECREF(attr); + Py_DECREF(cachedict); + Py_DECREF(obj_dict); + return NULL; + } + Py_DECREF(attr); + } else { + /* The wrapped object didn't have attribute attr_name. */ + PyErr_Clear(); + } + } + } + obj = (lru_cache_object *)type->tp_alloc(type, 0); if (obj == NULL) { Py_DECREF(cachedict); + Py_DECREF(obj_dict); return NULL; } @@ -1209,7 +1245,7 @@ lru_cache_new(PyTypeObject *type, PyObject *args, PyObject *kw) obj->lru_list_elem_type = state->lru_list_elem_type; Py_INCREF(cache_info_type); obj->cache_info_type = cache_info_type; - obj->dict = NULL; + obj->dict = obj_dict; obj->weakreflist = NULL; return (PyObject *)obj; } @@ -1421,6 +1457,28 @@ static int _functools_exec(PyObject *module) { _functools_state *state = get_functools_state(module); + state->list_of_lru_obj_attrs_to_clone = PyTuple_New(2); + if (state->list_of_lru_obj_attrs_to_clone == NULL) { + return -1; + } + { + PyObject *tmp; + PyObject *lru_attrs = state->list_of_lru_obj_attrs_to_clone; + if (lru_attrs == NULL) { + return -1; + } + tmp = PyUnicode_InternFromString("__defaults__"); + if (tmp == NULL || PyTuple_SetItem(lru_attrs, 0, tmp) != 0) { + return -1; + } + Py_DECREF(tmp); + tmp = PyUnicode_InternFromString("__kwdefaults__"); + if (tmp == NULL || PyTuple_SetItem(lru_attrs, 1, tmp) != 0) { + return -1; + } + Py_DECREF(tmp); + } + state->kwd_mark = _PyObject_CallNoArg((PyObject *)&PyBaseObject_Type); if (state->kwd_mark == NULL) { return -1; @@ -1475,6 +1533,7 @@ _functools_traverse(PyObject *module, visitproc visit, void *arg) Py_VISIT(state->partial_type); Py_VISIT(state->keyobject_type); Py_VISIT(state->lru_list_elem_type); + Py_VISIT(state->list_of_lru_obj_attrs_to_clone); return 0; } @@ -1486,6 +1545,7 @@ _functools_clear(PyObject *module) Py_CLEAR(state->partial_type); Py_CLEAR(state->keyobject_type); Py_CLEAR(state->lru_list_elem_type); + Py_CLEAR(state->list_of_lru_obj_attrs_to_clone); return 0; } From 941c63f2ca5de516a911e9920f702a09791fb1b8 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 1 May 2021 17:41:03 -0700 Subject: [PATCH 3/8] Fix the Python implementation and expand the test. --- Lib/functools.py | 6 ++++-- Lib/test/test_functools.py | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index b1f1fe8d9a6f27..ccc9fad1f56fa8 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -476,6 +476,8 @@ def _make_key(args, kwds, typed, return key[0] return _HashedSeq(key) +_LRU_CACHE_WRAPPER_ASSIGNMENTS = frozenset(WRAPPER_ASSIGNMENTS).union( + ('__defaults__', '__kwdefaults__')) def lru_cache(maxsize=128, typed=False): """Least-recently-used cache decorator. @@ -510,7 +512,7 @@ def lru_cache(maxsize=128, typed=False): user_function, maxsize = maxsize, 128 wrapper = _lru_cache_wrapper(user_function, maxsize, typed, _CacheInfo) wrapper.cache_parameters = lambda : {'maxsize': maxsize, 'typed': typed} - return update_wrapper(wrapper, user_function) + return update_wrapper(wrapper, user_function, assigned=_LRU_CACHE_WRAPPER_ASSIGNMENTS) elif maxsize is not None: raise TypeError( 'Expected first argument to be an integer, a callable, or None') @@ -518,7 +520,7 @@ def lru_cache(maxsize=128, typed=False): def decorating_function(user_function): wrapper = _lru_cache_wrapper(user_function, maxsize, typed, _CacheInfo) wrapper.cache_parameters = lambda : {'maxsize': maxsize, 'typed': typed} - return update_wrapper(wrapper, user_function) + return update_wrapper(wrapper, user_function, assigned=_LRU_CACHE_WRAPPER_ASSIGNMENTS) return decorating_function diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 68320c306a1df9..a6104547325c4d 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -1730,11 +1730,13 @@ def test_staticmethod(x): def test_lru_defaults_bug44003(self): @self.module.lru_cache(maxsize=None) - def func(arg='ARG', *, kw='KW'): + def func(arg='ARG', *, kw: str = 'KW'): return arg, kw + self.assertEqual(func.__wrapped__.__annotations__, {'kw': str}) self.assertEqual(func.__wrapped__.__defaults__, ('ARG',)) self.assertEqual(func.__wrapped__.__kwdefaults__, {'kw': 'KW'}) + self.assertEqual(func.__annotations__, {'kw': str}) self.assertEqual(func.__defaults__, ('ARG',)) self.assertEqual(func.__kwdefaults__, {'kw': 'KW'}) From c56cebecaad84d7dc0e7f0f4f90b29b11f4c56e1 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 1 May 2021 17:45:10 -0700 Subject: [PATCH 4/8] Remove incorrect type name from field name. --- Modules/_functoolsmodule.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 81e779b8392501..0b53f25f1462ae 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -19,7 +19,7 @@ typedef struct _functools_state { PyTypeObject *partial_type; PyTypeObject *keyobject_type; PyTypeObject *lru_list_elem_type; - PyObject *list_of_lru_obj_attrs_to_clone; + PyObject *lru_obj_attrs_to_clone; } _functools_state; static inline _functools_state * @@ -1198,9 +1198,9 @@ lru_cache_new(PyTypeObject *type, PyObject *args, PyObject *kw) /* Copy special attributes from the original function over to ours. */ { - Py_ssize_t n_attrs = PyTuple_GET_SIZE(state->list_of_lru_obj_attrs_to_clone); + Py_ssize_t n_attrs = PyTuple_GET_SIZE(state->lru_obj_attrs_to_clone); for (Py_ssize_t idx = 0; idx < n_attrs; ++idx) { - PyObject *attr_name = PyTuple_GET_ITEM(state->list_of_lru_obj_attrs_to_clone, idx); + PyObject *attr_name = PyTuple_GET_ITEM(state->lru_obj_attrs_to_clone, idx); if (attr_name == NULL) { Py_DECREF(cachedict); Py_DECREF(obj_dict); @@ -1457,13 +1457,13 @@ static int _functools_exec(PyObject *module) { _functools_state *state = get_functools_state(module); - state->list_of_lru_obj_attrs_to_clone = PyTuple_New(2); - if (state->list_of_lru_obj_attrs_to_clone == NULL) { + state->lru_obj_attrs_to_clone = PyTuple_New(2); + if (state->lru_obj_attrs_to_clone == NULL) { return -1; } { PyObject *tmp; - PyObject *lru_attrs = state->list_of_lru_obj_attrs_to_clone; + PyObject *lru_attrs = state->lru_obj_attrs_to_clone; if (lru_attrs == NULL) { return -1; } @@ -1533,7 +1533,7 @@ _functools_traverse(PyObject *module, visitproc visit, void *arg) Py_VISIT(state->partial_type); Py_VISIT(state->keyobject_type); Py_VISIT(state->lru_list_elem_type); - Py_VISIT(state->list_of_lru_obj_attrs_to_clone); + Py_VISIT(state->lru_obj_attrs_to_clone); return 0; } @@ -1545,7 +1545,7 @@ _functools_clear(PyObject *module) Py_CLEAR(state->partial_type); Py_CLEAR(state->keyobject_type); Py_CLEAR(state->lru_list_elem_type); - Py_CLEAR(state->list_of_lru_obj_attrs_to_clone); + Py_CLEAR(state->lru_obj_attrs_to_clone); return 0; } From 0efe3d64584f154c6ef40e5ed625a589d9ad36ef Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 1 May 2021 17:46:37 -0700 Subject: [PATCH 5/8] remove an unnecessary {} local scope. --- Modules/_functoolsmodule.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 0b53f25f1462ae..3e47fc82b6a5ea 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -1197,29 +1197,28 @@ lru_cache_new(PyTypeObject *type, PyObject *args, PyObject *kw) } /* Copy special attributes from the original function over to ours. */ - { - Py_ssize_t n_attrs = PyTuple_GET_SIZE(state->lru_obj_attrs_to_clone); - for (Py_ssize_t idx = 0; idx < n_attrs; ++idx) { - PyObject *attr_name = PyTuple_GET_ITEM(state->lru_obj_attrs_to_clone, idx); - if (attr_name == NULL) { + for (Py_ssize_t idx = 0; + idx < PyTuple_GET_SIZE(state->lru_obj_attrs_to_clone); + ++idx) { + PyObject *attr_name = PyTuple_GET_ITEM(state->lru_obj_attrs_to_clone, idx); + if (attr_name == NULL) { + Py_DECREF(cachedict); + Py_DECREF(obj_dict); + return NULL; + } + PyObject *attr = PyObject_GetAttr(func, attr_name); + if (attr != NULL) { + int err = PyDict_SetItem(obj_dict, attr_name, attr); + if (err != 0) { + Py_DECREF(attr); Py_DECREF(cachedict); Py_DECREF(obj_dict); return NULL; } - PyObject *attr = PyObject_GetAttr(func, attr_name); - if (attr != NULL) { - int err = PyDict_SetItem(obj_dict, attr_name, attr); - if (err != 0) { - Py_DECREF(attr); - Py_DECREF(cachedict); - Py_DECREF(obj_dict); - return NULL; - } - Py_DECREF(attr); - } else { - /* The wrapped object didn't have attribute attr_name. */ - PyErr_Clear(); - } + Py_DECREF(attr); + } else { + /* The wrapped object didn't have attribute attr_name. */ + PyErr_Clear(); } } From 1aea36b6be31eacbbc8b81e42d0f84e1b5309ec1 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 1 May 2021 17:47:37 -0700 Subject: [PATCH 6/8] Remove an unnecessary local. --- Modules/_functoolsmodule.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 3e47fc82b6a5ea..bef36cc49d35e3 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -1208,8 +1208,7 @@ lru_cache_new(PyTypeObject *type, PyObject *args, PyObject *kw) } PyObject *attr = PyObject_GetAttr(func, attr_name); if (attr != NULL) { - int err = PyDict_SetItem(obj_dict, attr_name, attr); - if (err != 0) { + if (PyDict_SetItem(obj_dict, attr_name, attr) != 0) { Py_DECREF(attr); Py_DECREF(cachedict); Py_DECREF(obj_dict); From 3cf8039ba435d0549f331a18f21d0c78955a4e48 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 1 May 2021 17:49:25 -0700 Subject: [PATCH 7/8] remove a redundant error check. --- Modules/_functoolsmodule.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index bef36cc49d35e3..50c5166b52da6e 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -1462,9 +1462,6 @@ _functools_exec(PyObject *module) { PyObject *tmp; PyObject *lru_attrs = state->lru_obj_attrs_to_clone; - if (lru_attrs == NULL) { - return -1; - } tmp = PyUnicode_InternFromString("__defaults__"); if (tmp == NULL || PyTuple_SetItem(lru_attrs, 0, tmp) != 0) { return -1; From 7347baa583ee906e91c0766d1628ac637514df1a Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 1 May 2021 18:15:18 -0700 Subject: [PATCH 8/8] The PyTuple API steals, unlike PyList... --- Modules/_functoolsmodule.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 50c5166b52da6e..0aa2cf0be28a5c 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -1463,15 +1463,13 @@ _functools_exec(PyObject *module) PyObject *tmp; PyObject *lru_attrs = state->lru_obj_attrs_to_clone; tmp = PyUnicode_InternFromString("__defaults__"); - if (tmp == NULL || PyTuple_SetItem(lru_attrs, 0, tmp) != 0) { + if (tmp == NULL || PyTuple_SetItem(lru_attrs, 0, tmp) != 0) { // steal return -1; } - Py_DECREF(tmp); tmp = PyUnicode_InternFromString("__kwdefaults__"); - if (tmp == NULL || PyTuple_SetItem(lru_attrs, 1, tmp) != 0) { + if (tmp == NULL || PyTuple_SetItem(lru_attrs, 1, tmp) != 0) { // steal return -1; } - Py_DECREF(tmp); } state->kwd_mark = _PyObject_CallNoArg((PyObject *)&PyBaseObject_Type);