bpo-40408: Fix support of nested type variables in GenericAlias. #19836
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@gvanrossum please take a look at this. |
serhiy-storchaka
commented
May 2, 2020
gvanrossum
reviewed
May 3, 2020
gvanrossum
reviewed
May 3, 2020
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
Comment on lines
250
to
251
| /* If obj is a generic alias, substitute type variables params | ||
| with substitutions argitems. */ |
There was a problem hiding this comment.
Suggested change
| /* 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.)
| else { | ||
| Py_INCREF(arg); | ||
| } | ||
| 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! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://bugs.python.org/issue40408