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

New analyzer: resolve name clashes between generated and existing nodes #6972

Merged
merged 1 commit into from Jun 12, 2019

Conversation

ilevkivskyi
Copy link
Member

@ilevkivskyi ilevkivskyi commented Jun 11, 2019

Fixes #6454
Fixes #6973

Note that the solution doesn't guarantee the order of numbers N in foo-redefinitionN coincides with the textual order of nodes. But it looks like we don't need this for anything.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Thanks for fixing some of worst remaining known crashes with the new semantic analyzer! Looks good.

@@ -290,6 +290,22 @@ def unmangle(name: str) -> str:
return name.rstrip("'")


def get_unique_redefinition_name(name: str, existing: Container[str]) -> str:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if the mangled names can be shown in error messages (say, when showing error context). If you believe that it might be the case, can you create a follow-up issue about it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Out of top of my head I found only one case (multiple inheritance) where we use names from symbol table instead of .name() or .fullname(), but there may be other. I will open an issue.

@JukkaL JukkaL merged commit 1fdc95b into python:master Jun 12, 2019
@ilevkivskyi ilevkivskyi deleted the new-analyzer-clash-fix branch June 12, 2019 13:56
PattenR pushed a commit to PattenR/mypy that referenced this pull request Jun 23, 2019
…es (python#6972)

Fixes python#6454.
Fixes python#6973.

Note that the solution doesn't guarantee the order of numbers N in 
foo-redefinitionN coincides with the textual order of nodes. But it looks 
like we don't need this for anything.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants