Skip to content

Conversation

@ilevkivskyi
Copy link
Member

Fixes #16473

Potentially we can turn this helper function into a proper visitor, but I don't think it is worth it as of right now.

@github-actions

This comment has been minimized.

@ilevkivskyi ilevkivskyi mentioned this pull request Nov 13, 2023
2 tasks
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

LGTM. One very minor code-style nit below:

Co-authored-by: Alex Waygood <[email protected]>
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@ilevkivskyi
Copy link
Member Author

Right before merging this, I noticed there is a very similar crash that this PR would not fix. It is a bit trickier to fix, but generally the logic is the same.

Note btw that deciding whether two recursive types overlap is non-trivial, for example in the case with two different recursive Dicts, the overlap is a single element (an empty dict), so we give an error, similar to regular dicts/lists etc. But in case with two different recursive Lists the overlap is infinite: all kinds of lists of nested empty lists, like [[], [[], [[[]]]], []], so we don't give an error. It is not completely obvious that it is a desired behavior, but it would be weird to complain about non-overlapping comparison for types with infinite overlap.

@github-actions
Copy link
Contributor

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

@ilevkivskyi ilevkivskyi merged commit 0699dde into python:master Nov 15, 2023
@ilevkivskyi ilevkivskyi deleted the fix-strict-eq-crash branch November 15, 2023 10:20
JukkaL pushed a commit that referenced this pull request Nov 22, 2023
Fixes #16473

Potentially we can turn this helper function into a proper visitor, but
I don't think it is worth it as of right now.

---------

Co-authored-by: Alex Waygood <[email protected]>
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.

1.7 Crash on recursive type

2 participants