Skip to content

New semantic analyzer: support recursive TypedDicts and NamedTuples #6445

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.

Sign up for GitHub

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

Jump to bottom
Closed
ilevkivskyi opened this issue Feb 20, 2019 · 7 comments · Fixed by #7124
Closed

New semantic analyzer: support recursive TypedDicts and NamedTuples #6445

ilevkivskyi opened this issue Feb 20, 2019 · 7 comments · Fixed by #7124
Assignees
@JukkaL
Labels
priority-0-high semantic-analyzer Problems that happen during semantic analysis topic-named-tuple topic-typed-dict

Comments

@ilevkivskyi
Copy link
Member

These have (limited) support in old analyzer, with new semantic analyzer they just crash.
@ilevkivskyi
Copy link
Member Author

There are some tests skipped with --no-new-semantic-analyzer that needs to be fixed.

@ilevkivskyi ilevkivskyi mentioned this issue Feb 20, 2019
Merged
@JukkaL
Copy link
Collaborator

JukkaL commented Apr 5, 2019

Recursive type aliases are similar.

@ilevkivskyi
Copy link
Member Author

I will take care of this issue.

@ilevkivskyi ilevkivskyi assigned ilevkivskyi and unassigned JukkaL Apr 5, 2019
@JukkaL
Copy link
Collaborator

JukkaL commented Jun 25, 2019

I have a work-in-progress branch that turns these into blocking errors.

@JukkaL JukkaL assigned JukkaL and unassigned ilevkivskyi Jun 25, 2019
JukkaL added a commit that referenced this issue Jun 27, 2019
@JukkaL
New semantic analyzer: enable tests related to recursively defined types
6489bb5 
There is a feature regression: we no longer expand recursive definition
once. I think that this is acceptable for now. We can either try to
replicate the expansion of recursive definitions later on, or we can
wait until we have full recursive support before we do anything about
this.

Only enabled tests that do something reasonable. Recursive type aliases
and other things still don't work.

Work towards #6445.
@JukkaL JukkaL mentioned this issue Jun 27, 2019
Merged
JukkaL added a commit that referenced this issue Jun 27, 2019
@JukkaL
New semantic analyzer: enable tests related to recursively defined ty…
9438211 
…pes (#7080)

There is a feature regression: we no longer expand recursive definition
once. I think that this is acceptable for now. We can either try to
replicate the expansion of recursive definitions later on, or we can
wait until we have full recursive support before we do anything about
this.

Only enabled tests that do something reasonable. Recursive type aliases
and other things still don't work.

Work towards #6445.
JukkaL added a commit that referenced this issue Jun 28, 2019
@JukkaL
New semantic analyzer: fix recursive type aliases
6f7cd9f 
This fixes maximum iteration count errors.

Recursive type aliases still don't work and the behavior is a
regression from the old semantic analyzer. Previously they were
truncated, but now they aren't supported at all. We could perhaps hack
something similar to what we used to have, but I don't think that it's
urgent.

This also breaks some use cases where "semi-recursive" type aliases
were supported previously. The support mechanism caused infinite
expansion for actual recursive type aliases. I couldn't come up with a
simple way to support both, so I decided to prioritize fixing infinite
expansion, as they have a worse impact on user experience.

Work towards #6445.
This was referenced Jun 28, 2019
Merged
Closed
JukkaL added a commit that referenced this issue Jul 1, 2019
@JukkaL
New semantic analyzer: fix recursive type aliases (#7091)
6f54a6f 
This fixes maximum iteration count errors.

Recursive type aliases still don't work and the behavior is a
regression from the old semantic analyzer. Previously they were
truncated, but now they aren't supported at all. We could perhaps hack
something similar to what we used to have, but I don't think that it's
urgent.

This also breaks some use cases where "semi-recursive" type aliases
were supported previously. The support mechanism caused infinite
expansion for actual recursive type aliases. I couldn't come up with a
simple way to support both, so I decided to prioritize fixing infinite
expansion, as they have a worse impact on user experience.

Work towards #6445.
@JukkaL
Copy link
Collaborator

JukkaL commented Jul 1, 2019

I think that this is now mostly resolved. Added #7111 as a follow-up issue.

@JukkaL JukkaL closed this as completed Jul 1, 2019
@ilevkivskyi
Copy link
Member Author

@JukkaL

I think that this is now mostly resolved. Added #7111 as a follow-up issue.

There are 7 tests that refer to this issue as an excuse, should we enable them first?

test-data/unit/check-classes.test:4358:# FIXME: #6445
test-data/unit/check-classes.test:4367:# FIXME: #6445
test-data/unit/check-classes.test:4375:# FIXME: #6445
test-data/unit/check-classes.test:4390:# FIXME: #6445
test-data/unit/check-unions.test:960:# https://github.com/python/mypy/issues/6445
test-data/unit/check-flags.test:1056:# Crashes on new semantic analyzer: https://github.com/python/mypy/issues/6445
test-data/unit/check-flags.test:1065:# Crashes on new semantic analyzer: https://github.com/python/mypy/issues/6445

@JukkaL JukkaL reopened this Jul 1, 2019
@JukkaL
Copy link
Collaborator

JukkaL commented Jul 1, 2019

Yeah, let's go through all of those issues firs.

JukkaL added a commit that referenced this issue Jul 2, 2019
@JukkaL
New semantic analyzer: update tests not to mention fixed issue
120cbaa 
These recursive type definitions no longer cause crashes.

Closes #6445.
@JukkaL JukkaL mentioned this issue Jul 2, 2019
Merged
JukkaL added a commit that referenced this issue Jul 2, 2019
@JukkaL
New semantic analyzer: fix bogus messages and update tests (#7124)
d174d75 
Various recursive type definitions no longer cause crashes.

Fix bogus messages about cyclic definitions when definition is
acyclic when there are also recursive type definitions in the same
target.

Closes #6445.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JukkaL JukkaL

Labels
priority-0-high semantic-analyzer Problems that happen during semantic analysis topic-named-tuple topic-typed-dict
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

New semantic analyzer: fix bogus messages and update tests python/mypy
2 participants
@JukkaL @ilevkivskyi