Skip to content

Fix crash on star unpacking to underscore#14624

Merged
hauntsaninja merged 6 commits intopython:masterfrom
ilevkivskyi:fix-star-crahs
Feb 8, 2023
Merged

Fix crash on star unpacking to underscore#14624
hauntsaninja merged 6 commits intopython:masterfrom
ilevkivskyi:fix-star-crahs

Conversation

@ilevkivskyi
Copy link
Copy Markdown
Member

@ilevkivskyi ilevkivskyi commented Feb 6, 2023

Fixes #14250

This one is interesting. It looks like most likely this was caused by my PR #7127 that fixed other crash. After looking a bit more, StarType is something old, and should never by used. At least I didn't find visit_star_type() in any of the type visitors. Actually mypy already uses assert False, if we get to a non-special-cased star expression.

Btw, I noticed that pythoneval test with empty expected output passes in case of a crash (at least on my machine), so I fix this too.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Great! Can we clean up all the StarType code then?

@ilevkivskyi
Copy link
Copy Markdown
Member Author

Can we clean up all the StarType code then?

Hm, it looks like yes, there are no more places where new (not copies) StarTypes are created.

Copy link
Copy Markdown
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Very nice!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 8, 2023

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@hauntsaninja hauntsaninja merged commit 891e035 into python:master Feb 8, 2023
@ilevkivskyi ilevkivskyi deleted the fix-star-crahs branch February 8, 2023 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash on nested unpacking of next() call

2 participants