Repository navigation
Fix leaking error messages during overload selection - #5211
Michael0x2a merged 1 commit into
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) |
| 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.
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
OK, this does fix the errors I was seeing in the Dropbox code base. Thanks for the quick fix! Let me know if you want to change the message suppressing logic now too -- if not feel free to merge yourself.
2 participants
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.