bpo-40408: Fix support of nested type variables in GenericAlias.#19836
Conversation
|
@gvanrossum please take a look at this. |
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
LG, just style suggestions.
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
Some more suggestions. Thanks for explaining LookupAttrId, I had forgotten its API. :-)
Objects/genericaliasobject.c
Outdated
| @@ -263,13 +308,19 @@ ga_getitem(PyObject *self, PyObject *item) | |||
| } | |||
| int is_tuple = PyTuple_Check(item); | |||
| Py_ssize_t nitem = is_tuple ? PyTuple_GET_SIZE(item) : 1; | |||
Objects/genericaliasobject.c
Outdated
| /* If obj is a generic alias, substitute type variables params | ||
| with substitutions argitems. */ |
There was a problem hiding this comment.
| /* If obj is a generic alias, substitute type variables params | |
| with substitutions argitems. */ | |
| /* If obj is a generic alias, substitute type variables params | |
| with substitutions argitems. For example, if obj is list[T], | |
| params is (T, S), and argitems is (str, int), return list[str]. | |
| If obj doesn't have a __parameters__ attribute or that's not | |
| a non-empty tuple, return obj, INCREF'ed. */ |
Objects/genericaliasobject.c
Outdated
| Py_INCREF(arg); | ||
| PyTuple_SET_ITEM(subargs, i, arg); | ||
| } | ||
| obj = PyObject_GetItem(obj, subargs); |
There was a problem hiding this comment.
Since this is the operation we've been preparing for, I'd set it apart with a blank line before and after.
| return NULL; | ||
| } | ||
| if (subparams && PyTuple_Check(subparams)) { | ||
| Py_ssize_t nparams = PyTuple_GET_SIZE(params); |
There was a problem hiding this comment.
This could be moved into the if (nsubargs != 0) { block below.
(Also the logic would be slightly simpler if the check for subparams being non-empty was part of the if (subparams && PyTuple_Check(subparams)) { check above.)
| Py_XDECREF(subparams); | ||
| } | ||
| PyTuple_SET_ITEM(newargs, iarg, arg); | ||
| } |
There was a problem hiding this comment.
Comment for the next line: (again) since it is the main event we've been working up to, offset it with a blank line before and after.
|
Thank you very much for your review! |
4 participants
https://bugs.python.org/issue40408