Do not blindly undefer on leaving fuction#18674
Conversation
This comment has been minimized.
This comment has been minimized.
|
Hm, some of the new errors are truly bizarre, I will need to dig deeper in this. |
|
OK, so here is the analysis for
cc @JukkaL who initially implemented this feature. IIUC possible fix is to save/restore full binder stack when checking deferred functions. Alternatively we may consider only deferring top-level functions, instead of the nested ones. IMO performance impact from this will be small.
I remember fixing something similar for the daemon was a huge pain (and I am not even sure we fixed it completely, see e.g. |
This comment has been minimized.
This comment has been minimized.
If we defer nested functions, that sounds like a problem. I think we should only defer top-level functions. |
Yeah, seems reasonable. More than two passes should only be needed in very rare circumstances. |
|
@JukkaL OK, I will create two small PRs for these two, and probably merge them first. IIUC it should be safer that other way around. |
This helps in rare cases, see discussion in #18674
This makes deferral logic more robust and more consistent with fine-grained mode. I also: * Change some terminology, as "top function" is ambiguous: top-level function vs top of stack function. * Update some docs and type annotations to match actual behavior (e.g. we do not defer lambdas) See also #18674 for some more motivation.
This comment has been minimized.
This comment has been minimized.
|
I decide also to put up #18723 (for |
This helps in rare cases, see discussion in python#18674
This makes deferral logic more robust and more consistent with fine-grained mode. I also: * Change some terminology, as "top function" is ambiguous: top-level function vs top of stack function. * Update some docs and type annotations to match actual behavior (e.g. we do not defer lambdas) See also python#18674 for some more motivation.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
OK, it looks like after merging all the "auxiliary" PRs we are fine here. |
Fixes #16496 for real.
The fix is trivial, just save and restore the previous value.