Skip to content

Don't have newly added files trigger a coarse-grained rebuild in fine-grained cache mode #4669

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

Merged
merged 4 commits into from
Mar 5, 2018

Conversation

msullivan
Copy link
Collaborator

No description provided.

@msullivan msullivan requested a review from JukkaL March 2, 2018 16:24
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.

This looks like a reasonable way to move forward with this, though we may want to reconsider the approach in the future. Looks good otherwise, but I'd like to see more test coverage around this, since this use case is likely not hit very often. Feel free to merge after you've addressed my comments.

@@ -971,3 +971,56 @@ x = Foo()
==
==
main:2: error: Too few arguments for "foo" of "Foo"

[case testAddModuleAfterCache1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to use the -skip-nocache suffix for some of these test cases?

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 don't see any need to. The main thing under test is cache behavior but they are still tests of module adding, which is subtle and could potentially find regressions in non-cache mode.

a.py:2: error: Too many arguments for "foo"
==

[case testAddModuleAfterCache3]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer to have more tests around this. For example, test that errors are correctly generated in all the new modules and existing modified modules, and make sure that we can correctly bind references to things defined in all three classes of modules:

  1. Cached but unmodified
  2. Cached and modified
  3. New modules

@msullivan msullivan merged commit ac90292 into master Mar 5, 2018
@gvanrossum gvanrossum deleted the add_optimiz branch March 5, 2018 20: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)
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