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

Preserve dirty set and add dirtiness assertion #2279

Merged
merged 2 commits into from
Oct 17, 2021
Merged

Conversation

pepeiborra
Copy link
Collaborator

@pepeiborra pepeiborra commented Oct 17, 2021

This PR is another spinoff from #2263

We want to preserve the dirty set across builds, since cancellation means that his-graph might not get a chance to register the previous dirty set.

We also add an assertion that "clean" keys are never a cache miss, which would have detected the bug. Assertions are disabled in -O builds by default, and the -fno-ignore-asserts is needed to enable them according to the GHC docs

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

assertions are great but it would be any chance to have some test to catch the breaking invariant?

@pepeiborra pepeiborra merged commit 6c647d1 into master Oct 17, 2021
@pepeiborra
Copy link
Collaborator Author

There are multiple ways that this invariant could be broken, and new ones could be introduced in the future. This is usually the case with stateful invariants like this one, very difficult to characterise with tests alone.

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