Fix leaking error messages during overload selection #5211
Conversation
Fixes python#5210 (?) When mypy tries selecting an appropriate overload alternative, it swaps out the current 'msg' object with a placeholder. If this placeholder does not accumulate any error messages, we know that alternative is a successful match. Previously, we replaced only checkexpr.ExpressionChecker's msg object with the placeholder. However, since checkexpr can sometimes call checker.TypeChecker, we need to replace its msg object as well to prevent error messages that would normally be silenced from leaking. More broadly, the current overload implementation broke the invariant that both msg fields refer to the same underlying MessageBuilder object. This pull request fixes that oversight.
|
@gvanrossum -- I think this fixes the bug you found earlier -- do you mind re-testing w/ these changes? Also, the way that I'm currently "capturing" the error messages feels pretty janky. Do you know if there's some better way of doing what the code is currently doing? (If not, I think we maybe should sometime in the future? It sort of feels like the codebase has several different ad-hoc ways of "capturing" or "conditionally silencing" errors and it might be nice to unify these different approaches in a more flexible way somehow -- maybe as part of a larger refactoring on how we handle error messages in general? /aside) |
| @@ -1238,7 +1238,9 @@ def infer_overload_return_type(self, | |||
| for typ in plausible_targets: | |||
| overload_messages = self.msg.clean_copy() | |||
| prev_messages = self.msg | |||
| assert self.msg is self.chk.msg | |||
There was a problem hiding this comment.
I think there's a slightly different pattern to ignore messages during a certain block of code, see e.g. checkmember.py L183. I'll test your code next.
Michael0x2a
deleted the
fix-leaking-error-messages-during-overload-selection
branch
Fixes #5210 (?)
When mypy tries selecting an appropriate overload alternative, it swaps out the current 'msg' object with a placeholder. If this placeholder does not accumulate any error messages, we know that alternative is a successful match.
Previously, we replaced only
checkexpr.ExpressionChecker's msg object with the placeholder. However, since checkexpr can sometimes callchecker.TypeChecker, we need to replace its msg object as well to prevent error messages that would normally be silenced from leaking.More broadly, the current overload implementation broke the invariant that both msg fields refer to the same underlying MessageBuilder object. This pull request fixes that oversight.