Scala 3.5.x stopped reporting generated classes via the Zinc incremental callback by default

[vasilmkd]

Compiler version

Scala 3.5.0-RC1 and above

I'm running Scala 3.5.0-RCX versions in the IntelliJ IDEA incremental compilation tests and I've noticed a change in behaviour. In IDEA, we have two modes of compilation, one using the Zinc incremental compiler, and one using the IDEA bytecode based incremental compiler.

Compiling using Zinc is not affected, because Zinc requires full analysis to run (xsbti.AnalysisCallback#enabled returns true).

When the alternative IDEA incremental compiler is used, we invoke scalac via sbt.internal.inc.AnalyzingCompiler. This is where I observe the change in behaviour.

All Scala versions prior to Scala 3.5.0 used to execute the xsbti.AnalysisCallback#generatedNonLocalClass and accompanying methods to report the production of .class files.

Judging by the previous behaviour, and the Scala 2 compiler source code, this method should be run regardless of the output of xsbti.AnalysisCallback#enabled. This can be seen here.

Somewhere around the changes made to the Scala 3 compiler when support for usePipelining was developed, this changed for Scala 3.5.0.

generatedNonLocalClass is now called only if xsbti.AnalysisCallback#enabled is true.

I will update this ticket as I discover more about the problem.

Tagging @bishabosha as someone who should have some context on the problem.

Thanks in advance.

[vasilmkd]

It's possible that this might be the commit.

Look for changes to ExtractAPI.scala.

[sjrd]

@bishabosha

[bishabosha]

so the breakdown of changes: #18137 (released in 3.4.0) introduced the change that ExtractAPI phase only runs if the callback is enabled, previously it only checked if the callback wasn't null

then commit a975398 (part of pipelining, so released in 3.5.0-RC1) moved notification of non-local classes from the backend phase, to the ExtractAPI phase - which due to the change above, means non-local classes were notified only when the callback is enabled.

A very simple fix would be to revert to running the ExtractAPI phase whenever the callback is merely present.

Otherwise we could restore reporting in backend when incremental phases are not enabled?

Or run a simplified collector in ExtractAPI to only collect classes for when we don't need incremental compile

[bishabosha]

@vasilmkd it would be good to know the behavior you observe with Scala 2, because it also only calls generatedNonLocalClass in the injected API phase, which as you showed, is only enabled when the callback is enabled

[vasilmkd]

When calling scalac through sbt.internal.inc.AnalyzingCompiler, we always pass a callback with enabled = false, regardless of the Scala version.
https://github.com/JetBrains/intellij-scala/blob/8e8495a31a79add1fb638060b81149b2d7db42ba/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/IdeaIncrementalCompiler.scala#L122

I have a test which runs some incremental compilation for many Scala versions. It only started failing when I added 3.5.0-RC1 to the mix.

https://github.com/JetBrains/intellij-scala/blob/8e8495a31a79add1fb638060b81149b2d7db42ba/scala/compiler-integration/test/org/jetbrains/plugins/scala/compiler/IncrementalCompilationTestBase.scala#L19-L33

Here's the test for reference.

@vasilmkd it would be good to know the behavior you observe with Scala 2, because it also only calls generatedNonLocalClass in the injected API phase, which as you showed, is only enabled when the callback is enabled

I'm now looking through Scala 2 code to verify

[vasilmkd]

Indeed, Scala 2.13 also doesn't run generatedNonLocalClass when enabled = false, I've verified that by manually debugging.

Let me then explain what the regression is in terms of the test expectations.

[vasilmkd]

The tests as can be seen here: https://github.com/JetBrains/intellij-scala/blob/8e8495a31a79add1fb638060b81149b2d7db42ba/scala/compiler-integration/test/org/jetbrains/plugins/scala/compiler/IncrementalCompilationTestBase.scala#L19-L33

org.jetbrains.plugins.scala.compiler.IncrementalCompilationTestBase#testRecompileOnlyAffectedFiles

A test project is set up with the following three source files.

// src/Fist.scala <- typo, but can be ignored
class First {
  def x = 1
}

// src/Second.scala
class Second extends First {
  println(x)
}

// src/Third.scala
class Third

The project is compiled as is, all .class files are produced.

The first source file is modified to be:

class First { def x = 1.0 }

The project is incrementally compiled. It is then asserted that First and Second (Second extends First) are recompiled, and Third is not recompiled.

The failure in this case is that Second.class and Second.tasty are not regenerated. It seems like Second.scala was not recompiled.

[vasilmkd]

org.jetbrains.plugins.scala.compiler.IncrementalCompilationTestBase#testDeleteOldTargetFiles

Again, a test project is created with the following sources:

src/First.scala
class First1
class First2

src/Second.scala
class Second

The project is compiled.
The source file src/First.scala is modified to (class First2 is removed):

class First1

The source file src/Second.scala is fully removed.
The project is incrementally compiled.

It is asserted that only First1.class and First1.tasty remain in the output directory, and that First2.class, First2.tasty, Second.class, Second.tasty have been removed.

With Scala 3.5.0-RCX, First2.class, First2.tasty, Second.class, Second.tasty all remain on disk. With all previous Scala versions, they are removed.

[vasilmkd]

org.jetbrains.plugins.scala.compiler.IncrementalCompilationTestBase#testDeleteTargetFilesForInvalidSources

A test project is created with the following sources:

// src/First.scala
class First

// src/Second.scala
class Second

The project is compiled.
The source file First.scala is modified to have a compiler error:

clas First // one 's' in clas

The project is incrementally compiled.

It is now asserted that First.class and First.tasty are removed while Second.class and Second.tasty remain. This is true for all Scala versions except 3.5.0-RCX. For Scala 3.5, First.class and First.tasty are not removed from disk.

Here is the list of tested Scala versions:

• Scala 2.10.6
• Scala 2.10.7
• Scala 2.11.0
• Scala 2.11.12
• Scala 2.12.0
• Scala 2.12.19
• Scala 2.13.14
• Scala 3.0.2
• Scala 3.1.3
• Scala 3.2.2
• Scala 3.3.3
• Scala 3.4.2
• Scala 3.5.0-RC3 (I also tested with all RCs)

[vasilmkd]

A very simple fix would be to revert to running the ExtractAPI phase whenever the callback is merely present.
Otherwise we could restore reporting in backend when incremental phases are not enabled?

If I'm allowed to speculate a bit. Maybe Scala 3 always implicitly depended on the existence of the callback in order to do reporting that Scala 2 might have been doing in the backend.

Your changes made enabled = true to be respected, which makes total sense and is supported by Scala 2, but now Scala 3 seems to be lacking some reporting on the backend.

[bishabosha]

If scala 2.13 is not calling the generatedNonLocalClass callback - then you must be observing something else to track this.

Scala 3 also has the CompilerCallback which you can observe all generated classes with onClassGenerated (and if you need to distinguish with local classes, then you can subtract any found via generatedLocalClass)

[bishabosha]

it could also depend which version of the scala 2.13 bridge sources you are compiling - I recall that pipelining was not present at 2.13.0, but added later - would be good to know if IntelliJ is using the latest source files possible - or even resolving the now published scala2-sbt-bridge jars since 2.13.12 - https://repo1.maven.org/maven2/org/scala-lang/scala2-sbt-bridge/

[bishabosha]

I'm happy to try the fix, but it would be good to verify why scala 2.13 is working when it doesn't call this method

[WojciechMazur]

@vasilmkd The fix was merged and is available in 3.6.0-RC1-bin-20240716-bbb45ca-NIGHTLY
Can you please test this version to check if the issue is fixed now before we start backporting these changes to 3.5.0-RC5 and 3.5.1. I would be grateful if you could take care of this quickly as we want to finish release 3.5.0 as soon as possible

[vasilmkd]

I'm on vacation until next Tuesday, 23.07.2024. I'll see what can be done.

[WojciechMazur]

I've cloned the https://github.com/JetBrains/intellij-scala and adapted it to run subclasses of IncrementalCompilationTestBase.
When using 3.5.0-RC4 it failed following tests

[error] Failed: Total 14, Failed 6, Errors 0, Passed 8
[error] Failed tests:
[error]         org.jetbrains.plugins.scala.compiler.IncrementalIdeaCompilationTest
[error]         org.jetbrains.plugins.scala.compiler.IncrementalIdeaOnServerCompilationTest
[error] (Test / testOnly) sbt.TestsFailedException: Tests unsuccessful

The tests execute successfully when using 3.6.0-RC1-bin-20240716-bbb45ca-NIGHTLY containing a fix. Based on that we're going to backport changes to 3.5.x.
However, the fact that incremental compilation tests are failing only when IncrementalityType.IDEA is used while it works correctly using IncrementalityType.SBT suggests a bug on the InteliJ side that should be fixed. PR that closed this issue was mostly merged under the pressure of already delayed release and I belive it should be reverted in the future along with a fix to the Scala 2.

[vasilmkd]

Thanks for checking. I also manually verified the change last week, so everything should be fine.

I agree that probably it will be good to revert/actually fix this both in Scala 2 and 3. But until things are fully understood by more people, it's totally fine to not introduce behaviour changes and I don't view it as pressure.