Skip to content

New semantic analyzer: track line numbers of placeholders #6983

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

Merged
merged 1 commit into from
Jun 13, 2019

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Jun 13, 2019

Line numbers are used sometimes to decide when a name should be looked
up from an outer scope, so they should be set for placeholders.

This actually breaks some forward references to assignment-based named
tuples. The reason is that we don't set becomes_typeinfo=True for
the placeholders. I'm going to open a separate issue about this, as
it's an existing issue and this change only exposes it.

Fixes #6949.

Line numbers are used sometimes to decide when a name should be looked
up from an outer scope, so they should be set for placeholders.

This actually breaks some forward references to assignment-based named
tuples. The reason is that we don't set `becomes_typeinfo=True` for
the placeholders. I'm going to open a separate issue about this, as
it's an existing issue and this change only exposes it.

Fixes #6949.
@JukkaL JukkaL requested a review from ilevkivskyi June 13, 2019 10:55
Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

Thanks! This all makes sense. Would it be better to first merge fix the named tuple issue to avoid changing the test?

@JukkaL
Copy link
Collaborator Author

JukkaL commented Jun 13, 2019

Would it be better to first merge fix the named tuple issue to avoid changing the test?

I think that the named tuple issue is lower priority, since forward references to named tuples aren't supported on the old semantic analyzer, so it's not a regression. Also, the fix may be quite involved and may involve changing how other things such as type aliases and NewTypes are processed.

@JukkaL JukkaL merged commit 00b3a0a into master Jun 13, 2019
@JelleZijlstra JelleZijlstra deleted the semanal-scope branch June 13, 2019 16:34
PattenR pushed a commit to PattenR/mypy that referenced this pull request Jun 23, 2019
Line numbers are used sometimes to decide when a name should be looked
up from an outer scope, so they should be set for placeholders.

This actually breaks some forward references to assignment-based named
tuples. The reason is that we don't set `becomes_typeinfo=True` for
the placeholders. I'm going to open a separate issue about this, as
it's an existing issue and this change only exposes it.

Fixes python#6949.
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.

New semantic analyzer: cannot resolve assignment to imported value from outer scope
2 participants