Skip to content

Refactor init checker: Extract reusable code #16705

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
Jan 29, 2023

Conversation

liufengyun
Copy link
Contributor

Refactor init checker: Extract reusable code

@liufengyun liufengyun requested a review from olhotak January 16, 2023 23:05
@liufengyun liufengyun marked this pull request as ready for review January 17, 2023 00:02
*
* `changed == false` implies that the fixed point has been reached.
*/
protected var changed: Boolean = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Is changed part of the cache? Is the heap now intended to be independent of the cache? Should changed and hasChanged be renamed to indicate that they are about the heap?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, changed is part of the cache, it's not related to the heap.

Copy link
Contributor

@olhotak olhotak left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

* cache.last = cache.current
* cache.current = Empty
* cache.changed = false
* iterate(outputCache, emptyCache)
Copy link
Contributor

Choose a reason for hiding this comment

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

If iterate is a recursive call to def iterate above, then def iterate should have these two explicit parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's changed in the last commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, sorry, yes, you're right.

@olhotak olhotak merged commit b8ad7b1 into scala:main Jan 29, 2023
@olhotak olhotak deleted the refactor-init branch January 29, 2023 20:31
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