Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash on unimported Any with Required/NotRequired #17609

Closed
wants to merge 1 commit into from

Conversation

andersk
Copy link
Contributor

@andersk andersk commented Jul 30, 2024

(To reproduce the crash without mypyc, replace cast(ProperType, typ) with an assertion in get_proper_type.)

Fixes python#17604; fixes python#17608.  (To reproduce the crash without mypyc,
replace `cast(ProperType, typ)` with an assertion in
`get_proper_type`.)

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

self.msg.unimported_type_becomes_any("Type of variable", s.type, s)
self.msg.unimported_type_becomes_any(
"Type of variable",
s.type.item if isinstance(s.type, RequiredType) else s.type,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to use some existing function to covert RequiredType to ProperType? Like get_proper_type? Or maybe something similar. Right now it feels too specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_proper_type is already being called—that’s where it’s crashing. It doesn’t convert RequiredType. Should it? That seems like a much bigger change though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about it :)
cc @JukkaL and @ilevkivskyi

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is not a fix, but a horrible dirty hack. I am actually going to revert another recent crash "fix" by @andersk and replace it with a proper one when I will have time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to have offended you!

@ilevkivskyi
Copy link
Member

Superseded by #17640

@ilevkivskyi ilevkivskyi closed this Aug 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants