fix(python): type-checking may require incorrect type #3820
Conversation
Since dae724c, Python type-checking relies on a nested stub function as a type annotations source. The parameter signature of that stub was copied verbatim from the surrounding function, including any forward type references. However, the forward references can safely be replaced with regular type references as the module is guaranteed to be fully loaded by the time the stub is created, and using forward-references there results in `typeguard` possibly evaluating those in a different context than the one where the surrounding function was defined. The consequence of this is that multiple different foward references by the same name may be incorrectly treated as referring to the same type, despite coming from different modules. This is fixed by turning forward type references in the stub with regular type references (in other words, removing any `"` from the parameter signature of the stub), which lets the type be resolved from the local definition context instead of the final runtime context in which the function is called. Fixes #3818
|
Romain indicated to me in private chat that he believes this will not cause performance impact. Don't want to hold up the fix if it's legit, since people are waiting for it. |
|
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
|
Merging (with squash)... |
I have no infrastructure to produce a reliable, comparable, reproductible benchmark of load times. But I could give it a shot in a somewhat un-scientific manner... But what are you concerned about here? That the new code will take significantly longer to load than the previous code? I don't see how this could be the case (or else, the Python VM would be utter rubbish, which it isn't)
There is already one added as part of the PR in |
Since dae724c, Python type-checking
relies on a nested stub function as a type annotations source. The
parameter signature of that stub was copied verbatim from the
surrounding function, including any forward type references.
However, the forward references can safely be replaced with regular type
references as the module is guaranteed to be fully loaded by the time
the stub is created, and using forward-references there results in
typeguardpossibly evaluating those in a different context than theone where the surrounding function was defined. The consequence of this
is that multiple different foward references by the same name may be
incorrectly treated as referring to the same type, despite coming from
different modules.
This is fixed by turning forward type references in the stub with
regular type references (in other words, removing any
"from theparameter signature of the stub), which lets the type be resolved from
the local definition context instead of the final runtime context in
which the function is called.
Fixes #3818
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.