Skip to content

New semantic analyzer: should we collect functions from AST instead of symbol tables? #6454

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 · 2 comments · Fixed by #6972
Closed

New semantic analyzer: should we collect functions from AST instead of symbol tables? #6454

ilevkivskyi opened this issue Feb 20, 2019 · 2 comments · Fixed by #6972
Labels
needs discussion priority-1-normal semantic-analyzer Problems that happen during semantic analysis

Comments

@ilevkivskyi
Copy link
Member

Currently, functions are collected for processing from symbol tables, not form ASTs. This causes some issues, for example, if there is method name clash in class-based named tuple, then the user provided method will be not analyzed, which will cause a crash later, see testNewNamedTupleIllegalNames.

This is a follow-up for #6452
@ilevkivskyi ilevkivskyi added needs discussion priority-1-normal semantic-analyzer Problems that happen during semantic analysis labels Feb 20, 2019
@JukkaL
Copy link
Collaborator

JukkaL commented Feb 22, 2019

This is something I started to wonder about as well. This may also affect fine-grained incremental mode, and it would be nice if the solution we come up would support that as well. I see three plausible alternatives for how to deal with this:

  • Rename definitions with name clashes. For example, we can use dummy suffixes, as we currently do for certain things. We'd need to extend this to deal with methods at least, and to make sure the renamed functions work consistently in every context.
  • Traverse the module top level to find all functions, as suggested above. This may not work well in the context of mypy daemon, however.
  • Treat functions with name conflicts as part of the module top level instead of as independent functions.

@ilevkivskyi ilevkivskyi mentioned this issue Mar 6, 2019
Merged
ilevkivskyi added a commit that referenced this issue Mar 7, 2019
@ilevkivskyi
Fix dataclass plugin to work with new semantic analyzer (#6515)
892b8d8 
Fixes #6324

This enables two test files that previously had few crashes.

Changes in this PR:
* Add `defer()` to public plugin API, otherwise dataclasses will not works with forward references.
* Update one plugin test to use the new analyzer because the output is a bit more verbose, and probably not worth the hassle of making it identical to old analyzer.
* One test is skipped because of existing issue #6454
* Add the generated type variable used for self types to the class body (this is identical to named tuples and `attrs` plugin).
@ilevkivskyi ilevkivskyi mentioned this issue May 6, 2019
Closed
@JelleZijlstra
Copy link
Member

#6783 has a concrete example of a crash with dataclasses that's caused by this.

@ilevkivskyi ilevkivskyi mentioned this issue Jun 11, 2019
Merged
JukkaL pushed a commit that referenced this issue Jun 12, 2019
@ilevkivskyi @JukkaL
New analyzer: resolve name clashes between generated and existing nod…
1fdc95b 
…es (#6972)

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.
PattenR pushed a commit to PattenR/mypy that referenced this issue Jun 23, 2019
@ilevkivskyi @PattenR
New analyzer: resolve name clashes between generated and existing nod…
bf94169 
…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
Assignees
No one assigned
Labels
needs discussion priority-1-normal semantic-analyzer Problems that happen during semantic analysis
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

New analyzer: resolve name clashes between generated and existing nodes ilevkivskyi/mypy
3 participants
@JelleZijlstra @JukkaL @ilevkivskyi