Do not cache fast container types inside lambdas#20166
Do not cache fast container types inside lambdas#20166hauntsaninja merged 1 commit intopython:masterfrom
Conversation
sterliakov
changed the title
|
Diff from mypy_primer, showing the effect of this PR on open source code: spark (https://github.com/apache/spark)
- python/pyspark/core/rdd.py:2210: error: Unused "type: ignore" comment [unused-ignore]
|
|
And that's correct, pyspark error is not a false positive - |
| if not self.in_lambda_expr: | ||
| # We cannot cache results in lambdas - their bodies can be accepted in | ||
| # error-suppressing watchers too early | ||
| self.resolved_type[e] = ct |
There was a problem hiding this comment.
Isn't there an expr cache that stores errors too? Is it possible to use that instead?
There was a problem hiding this comment.
We already explicitly bypass the expr cache for lambdas, so probably no?
Lines 6035 to 6050 in 843d133
There was a problem hiding this comment.
That makes some sense, but I don't quite get the comment there. I also don't get why this extra cache exists...
I guess the comment is saying "the same expr can be evaluated multiple times in different contexts" which makes a bit of sense?
There was a problem hiding this comment.
I also do not fully understand why a separate resolved_type storage is needed at all. It might well be just a remainder that wasn't cleaned up when expr_cache was introduced, but I'm not certain.
This lambda special-casing is coming from in #19408 and #19505, we discovered it independently. lambda exprs are handled in a completely different way with and without type context (see infer_lambda_type_using_context and branching on its result). There's a big difference between accepting a ReturnStmt and its expression alone - supporting both ways essentially means that we use different context stack entries on different paths for the same expression. This part is a bit difficult to reason about, but I still hope I got it correctly...
There was a problem hiding this comment.
OK, I guess it would make sense to try removing resolved_type cache in a followup...
|
Hey, thanks for working on this! |
4 participants
Fixes #20163.
fast_container_typeuses another layer of expression cache, it also has to be bypassed from within lambdas.