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

[mypyc] Fix using values from other modules that were reexported #7496

Merged
merged 2 commits into from
Sep 11, 2019

Conversation

msullivan
Copy link
Collaborator

@msullivan msullivan commented Sep 10, 2019

There are a couple parts to this:

  • Compile module attribute accesses by compiling the LHS on its own
    instead of by loading its fullname (which will be what mypy thinks
    its source is)
  • Only use the static modules if they have actually been imported

Closes mypyc/mypyc#393.

I ran into this again while finishing up my separate compilation PR and so got annoyed and fixed it.

There are a couple parts to this:
 * Compile module attribute accesses by compiling the LHS on its own
   instead of by loading its fullname (which will be what mypy thinks
   its source is)
 * Only use the static modules if they have actually been imported

Closes #393.
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 this! I'm glad we won't need to use the workaround any more. Just a few minor comments, otherwise looks good.

self.imports = [] # type: List[str]
# We use an OrderedDict for this so we can maintain insertion order
# and do quick lookups
self.imports = OrderedDict() # type: OrderedDict[str, None]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Document this attribute. Maybe this would benefit from renaming -- it took me a while to figure out what it's used for.

@@ -4645,3 +4645,18 @@ from native import foo

assert foo(None) == None
assert foo([1, 2, 3]) == ((1, 2, 3), [1, 2, 3])

[case testReexport]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe also test re-exporting a module or a class. What about re-exporting a global variable?

@msullivan msullivan merged commit 1ef02bb into master Sep 11, 2019
@msullivan msullivan deleted the imports branch September 11, 2019 22:58
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.

Problems using reexported values
2 participants