Repository navigation
New semantic analyzer: don't add submodules to symbol tables - #7005
Conversation
This affected stubs for requests.
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
Thanks for the cleanup, looks good! I have couple requests for tests/comments.
Also edit the PR description before merging otherwise this will close #6828.
| else: | ||
| if self.is_incomplete_namespace(file.fullname()): | ||
| self.record_incomplete_ref() | ||
| # Handle 'module.foo'. |
There was a problem hiding this comment.
Doesn't this comment just duplicate the one two lines above?
There was a problem hiding this comment.
Removed one of the comments.
| # Rebind potential references to old version of current module in | ||
| # fine-grained incremental mode. | ||
| if module == self.cur_mod_id: | ||
| names = self.globals |
There was a problem hiding this comment.
Since we are here again, could you please add a test that would fail without these two lines. Currently all tests pass if I remove them.
There was a problem hiding this comment.
I couldn't come up with a test that would require this. It's possible that fine-grained dependencies will deal with all the cases that this was supposed to handle. I've removed this now.
| var_type = AnyType(TypeOfAny.from_unimported_type) | ||
| v = Var(name, type=var_type) | ||
| v._fullname = fullname | ||
| sym = SymbolTableNode(GDEF, v) |
There was a problem hiding this comment.
Maybe add a comment explaining why we create a new Var every time for technically the same missing module? (IIUC the logic is the same as explained in the docstring for create_getattr_var()).
…7005) Previously we added each submodule implicitly to the the symbol table of the parent package. This PR removes this and instead we look up names from the modules dictionary if they aren't found in the symbol table. The change only affects the new semantic analyzer. This provides a foundation that should make python#6828 much easier to address. It also arguably cleans up the code. Also refactor some related code to avoid duplication. This isn't a pure refactor since some error messages are slightly different.
2 participants
Previously we added each submodule implicitly to the the symbol table of
the parent package. This PR removes this and instead we look up names from
the modules dictionary if they aren't found in the symbol table. The change
only affects the new semantic analyzer.
This provides the foundation that should make it easier to fix #6828. It also
arguably cleans up the code.
Also refactor some related code to avoid duplication.
This isn't a pure refactor since some error messages are slightly different.