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

Include extra invalidations in initial validation to fix initial compilation error #1284

Merged
merged 6 commits into from
Nov 18, 2023

Conversation

Friendseeker
Copy link
Member

During initial invalidation, if a class depends on invalidated class, and some invalidated class depends on that class, also invalidate that class.

Fixes #598.

To validate the fix, minimal reproduction of the issue in #598 (comment) and #654 is included as zinc scripted test.

Since comment says "This should compile" and the test expects successful compilation after clean, I guess the incremental compilation is indeed not supposed to fail there?
@Friendseeker Friendseeker changed the title Include extra invalidations in initial validation to fix compiler error Include extra invalidations in initial validation to fix initial compilation error Nov 17, 2023
Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

Thanks!

@eed3si9n eed3si9n merged commit 94c0b0c into sbt:develop Nov 18, 2023
7 checks passed
@Friendseeker Friendseeker deleted the trait-class-bug-fix branch November 18, 2023 04:15
@smarter
Copy link
Contributor

smarter commented Nov 18, 2023

Great work, thank you!

@SethTisue
Copy link
Member

Wow, this closed a lot of tickets! 👏

eed3si9n pushed a commit that referenced this pull request Oct 17, 2024
Ref #1284
Fixes #1461
Fixes #1420
Also fixes com-lihaoyi/mill#3748 downstream

Not sure if there's a better way to fix this? Just opening this to spark discussion

The original bugs are fine, but the solution seems incorrect, and is both overly conservative (invalidating everything based on whitespace?) and not conservative enough (doesn't handle cycles with length > 2?).
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.

Changing abstract class to trait causes "invalid superClass"
4 participants