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

Fine-grained: Support NewType and reset subtype caches #4656

Merged
merged 2 commits into from
Mar 2, 2018
Merged

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Mar 1, 2018

NewType work highlighted an issue with subtype caches with stale
information leaking, and this fixes that issue as well. We reset the
subtype cache in two places:

  • When calculating the MRO; we reset caches of all base classes as
    well.

  • When merging a new version of a TypeInfo, which may have a
    different MRO; we reset all caches of base classes in the old
    MRO, as they might no longer be supertypes.

NewType work highlighted an issue with subtype caches with stale
information leaking, and this fixes that issue as well. We reset the
subtype cache in two places:

* When calculating the MRO; we reset caches of all base classes as
  well.

* When merging a new version of a TypeInfo, which may have a
  different MRO; we reset all caches of base classes in the old
  MRO, as they might no longer be supertypes.
Copy link
Collaborator

@msullivan msullivan left a comment

Choose a reason for hiding this comment

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

One question, but looks good

@@ -2122,6 +2127,7 @@ def calculate_mro(self) -> None:
self.is_enum = self._calculate_is_enum()
# The property of falling back to Any is inherited.
self.fallback_to_any = any(baseinfo.fallback_to_any for baseinfo in self.mro)
self.reset_subtype_cache()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't fully understand why this is necessary (though it certainly isn't /wrong/). Adding a new subclass shouldn't invalidate any of the caches, since they store only positive information?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that some changes in generic base classes may require this. Say, originally the base class was C[A] and after update it is C[B]. This would change the subtype relation even though the MRO is unchanged. If we are doing a refresh of the class there would be no AST merge so without this the subtype cache would not get emptied.

@JukkaL JukkaL merged commit 6f9723c into master Mar 2, 2018
@gvanrossum gvanrossum deleted the fg-newtype branch March 2, 2018 21:11
carljm added a commit to carljm/mypy that referenced this pull request Mar 6, 2018
* master:
  New files shouldn't trigger a coarse-grained rebuild in fg cache mode (python#4669)
  Bump version to 0.580-dev
  Update revision history for 0.570 (python#4662)
  Fine-grained: Fix crashes when refreshing synthetic types (python#4667)
  Fine-grained: Support NewType and reset subtype caches (python#4656)
  Fine-grained: Detect changes in additional TypeInfo attributes (python#4659)
  Fine-grained: Apply semantic analyzer patch callbacks (python#4658)
  Optimize fine-grained update by using Graph as the cache (python#4622)
  Cleanup check_reverse_op_method (python#4017)
  Fine-grained: Fix AST merge issues (python#4652)
  Optionally check that we don't have duplicate nodes after AST merge (python#4647)
yedpodtrzitko pushed a commit to kiwicom/mypy that referenced this pull request Mar 15, 2018
NewType work highlighted an issue with subtype caches with stale
information leaking, and this fixes that issue as well. We reset the
subtype cache in two places:

* When calculating the MRO; we reset caches of all base classes as
  well.

* When merging a new version of a TypeInfo, which may have a
  different MRO; we reset all caches of base classes in the old
  MRO, as they might no longer be supertypes.
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.

2 participants