New semantic analyzer: Keep explicit type and inferred status in sync #6988
Conversation
mypy/newsemanal/semanal.py
Outdated
| @@ -2118,6 +2118,8 @@ def unwrap_final(self, s: AssignmentStmt) -> bool: | |||
| return False | |||
| lval = s.lvalues[0] | |||
| assert isinstance(lval, RefExpr) | |||
| # Reset inferred status if it was set due to simple literal rvalue on previous iteration. | |||
| lval.is_inferred_def = s.type is None | |||
There was a problem hiding this comment.
This seems error-prone and hard to reason about. For example, what happens if there are multiple assignments to a final name? How hard it would be to avoid the need to reset this (by not storing the bad value perhaps)?
There was a problem hiding this comment.
What do you mean by "bad" value? The value of is_inferred_def is set correctly, but just few lines above we reset s.type to None, but not the flag. If we don't reset it together, s.type will stay None.
For example, what happens if there are multiple assignments to a final name?
There will be an inconsistency with old analyzer. But we anyway give an error. Also this can be fixed only doing the reset if lvalue.is_new_def.
Anyway, if you have a better fix, go ahead.
|
After all we decided to go with a quick-fix. |
|
@JukkaL does this look acceptable? |
python#6988) This is (kind of) an actual fix for python#6952 The problem is that previously `unwrap_final()` unconditionally set `s.type` to `None` for bare `Final`, even if the type was inferred from a simple literal r.h.s. on a previous iteration. At the same time `lvalue.is_inferred_def` remained `False`, so that the type was not inferred on the second iteration. This way `s.type` and `lvalue.is_inferred_def` may get out of sync depending on the number of iterations, thus causing weird behavior in type checker. I generally can say the current way of storing the inferred status is error-prone, but we already have python#6458 to track this.
This is (kind of) an actual fix for #6952
The problem is that previously
unwrap_final()unconditionally sets.typetoNonefor bareFinal, even if the type was inferred from a simple literal r.h.s. on a previous iteration. At the same timelvalue.is_inferred_defremainedFalse, so that the type was not inferred on the second iteration.This way
s.typeandlvalue.is_inferred_defmay get out of sync depending on the number of iterations, thus causing weird behavior in type checker.I generally can say the current way of storing the inferred status is error-prone, but we already have #6458 to track this.