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

Better documentation needed for includeTransitiveInitialInvalidations #1313

Open
Friendseeker opened this issue Dec 22, 2023 · 2 comments
Open

Comments

@Friendseeker
Copy link
Member

Friendseeker commented Dec 22, 2023

* Returns the invalidations that are the result of the `currentInvalidations` + the
* `previousInvalidations` that depend transitively on `currentInvalidations`.
*
* We do this step on every incremental compiler iteration of a project where
* `previousInvalidations` typically refers to the classes invalidated in the
* previous incremental compiler cycle.
*
* @param previousInvalidations
* @param currentInvalidations
* @param findClassDependencies
* @return
*/
private[this] def includeTransitiveInitialInvalidations(
previousInvalidations: Set[String],
currentInvalidations: Set[String],
findClassDependencies: String => Set[String]
): Set[String] = {
val newInvalidations = currentInvalidations -- previousInvalidations
log.debug(s"New invalidations:${ppxs(newInvalidations)}")
val newTransitiveInvalidations =
IncrementalCommon.transitiveDeps(newInvalidations, log)(findClassDependencies)
// Include the initial invalidations that are present in the transitive new invalidations
val reInvalidated = previousInvalidations.intersect(newTransitiveInvalidations)
log.debug(
s"Previously invalidated, but (transitively) depend on new invalidations:${ppxs(reInvalidated)}"
)
newInvalidations ++ reInvalidated
}

Issue: It is hard to tell the purpose of reInvalidated. Ideally the need to include reInvalidated can be documented better.

Preferably we can also add a unit test to demonstrate why reInvalidated needs to be returned. Currently, if we only return newInvalidations, every zinc scripted test still passes.

@eed3si9n
Copy link
Member

So something like

  1. User change invalidates A1.scala
  2. By some simple invalidation, A1.scala invalidates B1.scala, like trait inheritance. Maybe it changes the API of the code somehow?
  3. B1.scala invalidates another code C1.scala due to the API change. This also changes the API of the code somehow?
  4. This then invalidates A1.scala, maybe because one of the methods references the change API in C1.scala

We'd need to recompile A1.scala with *.class of new C1.scala on the classpath.

@Friendseeker
Copy link
Member Author

Thanks. I will look into that case. I don't fully understand why A1.scala would not be invalidated anyways in the next cycle when invalidateAfterInternalCompilation is called. But I guess playing with an active debugger would answer all my questions.

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

No branches or pull requests

2 participants