More JVM backend cleanup by SolalPirelli · Pull Request #25747 · scala/scala3

SolalPirelli · 2026-04-09T14:58:49Z

Review commit by commit, each of them individually passes tests (I think, I did some light rebasing).

Like the rest of the compiler, use Context in methods, do not capture it inside classes
- Includes splitting CoreBTypes into BTypeLoader and WellKnownBTypes; the latter is what CoreBTypesFromSymbols used to be, minus loading stuff that went into the former. Only the latter captures a Context (and it's the only remaining class to do so).
- This means we can stop modifying (!) the Context in GenBCode
- We also no longer need to lock while calling genClassDef (it isn't supposed to be multi-threaded anyway, but right now on main if you remove the lock all hell breaks loose)
Remove remaining JVM backend references from the frontend.
Simplify code, delete dead code, etc.

We can't quite remove frontendLock / PPFA yet, as this happens if we do:

[info] Test dotty.tools.dotc.CompilationTests.parallelBackend started
[                                        ] completed (0/7, 0 failed, 1s)Error while emitting gopher/OChannel
Cyclic reference involving class LambdaDeserialize

 Run with -explain-cyclic for more details.
Compilation failed for: 'compiling 'tests/pos/i10477' in test 'parallelBackend''
[=================================>      ] completed (6/7, 1 failed, 4s)Error while emitting adts/t2$Option$
Cyclic reference involving class LambdaMetafactory

 Run with -explain-cyclic for more details.
[=================================>      ] completed (6/7, 1 failed, 4s)Compilation failed for: 'compiling 'tests/pos/reference' in test 'parallelBackend''
[=======================================>] completed (7/7, 2 failed, 4s)

How much have you relied on LLM-based tools in this contribution?

not

How was the solution tested?

existing tests, this is a pure refactoring

SolalPirelli · 2026-04-10T12:35:35Z

 import java.util.ConcurrentModificationException

 object FileWriters {
-  type InternalName = String


The existence of BTypes.InternalName is already questionable, having a dupe here is really weird IMHO

(This file already does its own stuff too much, like a unique "ReadOnlyContext" abstraction...)

SolalPirelli · 2026-04-10T12:36:02Z


 object FileWriters {
-  type InternalName = String
-  type NullableFile =  AbstractFile | Null


The backend needs to always get a file back, and only the JAR writer didn't do that here, so I copied the code from the backend version, and thus we no longer have nullable files

SolalPirelli · 2026-04-10T12:36:15Z

-  type InternalName = String
-  type NullableFile =  AbstractFile | Null
+  private def classRelativePath(className: String, suffix: String): String =
+    className.replace('.', '/') + suffix


Moved up because it's used by both the tasty and classfile writers

SolalPirelli · 2026-04-10T12:38:35Z

  }

-}
+  /**


Review with "ignore whitespace" on

SolalPirelli · 2026-04-13T14:16:10Z

 import dotty.tools.dotc.core.StdNames.nme

-final class BoxUnbox(backendUtils: BackendUtils, callGraph: CallGraph, ts: CoreBTypes) {
+final class BoxUnbox(backendUtils: BackendUtils, callGraph: CallGraph, ts: WellKnownBTypes) {


This is a large commit but it's just renaming and moving stuff. Anything loading is in the loader, any known types are in WellKnownBTypes, though the loader also has Object + Null + Nothing well-known types since it needs them as part of loading (and WellKnownBTypes just forwards)

SolalPirelli · 2026-04-15T13:37:47Z

-        Some(classNode, moduleNode)
-      } catch {
-        case ex: Exception =>
-          frontendAccess.optimizerWarning(em"Error while reading InlineInfoAttribute: ${ex.getMessage}", fullName, NoSourcePosition)


I don't think this is a useful "warning". It's a try/catch around the entire classloading for a hypothetical problem reading one small part of it. If this loading fails we should fail, imho.

I think the idea is that we didn't want to fail the compiler just because something could not be inlined / optimized. Maybe the warning text ("while reading InlineInfoAttribute") is just misleading?

Fair, let's reuse the existing "class not found" return value infrastructure then

SolalPirelli · 2026-04-23T13:21:11Z

Rebased over the latest changes in main (mostly the unsafeNulls removal outside of the backend, and the entry point collection removal)

Now the only non-backend file referencing the JVM backend is Compiler.scala, as it should be.

lrytz

Looks good, a lot of nice cleanups!

lrytz · 2026-04-28T11:45:05Z

-        Some(classNode, moduleNode)
-      } catch {
-        case ex: Exception =>
-          frontendAccess.optimizerWarning(em"Error while reading InlineInfoAttribute: ${ex.getMessage}", fullName, NoSourcePosition)


I think the idea is that we didn't want to fail the compiler just because something could not be inlined / optimized. Maybe the warning text ("while reading InlineInfoAttribute") is just misleading?

lrytz · 2026-04-28T11:52:54Z

      }
-      // Don't try to build a BType for `VarHandle`, it doesn't exist on JDK 8
-      if (owner.name == ts.jliMethodHandleRef.internalName || owner.name == "java/lang/invoke/VarHandle")
+      if (owner.name == "java/lang/invoke/MethodHandle" || owner.name == "java/lang/invoke/VarHandle")


In general we should try to always use a BType instance. The reason is the getCommonSuperClass method, which is invoked by ASM during bytecode writing for computing stack map frames.

The comment in getCommonSuperClass mentions "see [[cachedClassBType]]", which was renamed in one of the refactorings in scala3, it's "classBTypeFromInternalName" now, but I think the principle is the same. BType lookup at this stage will only succeed if the BType was previously constructed while running the backend / optimizer.

If it can't be done here, I would at least add a code comment about it.

So the idea is that if owner.name == ts.jliMethodHandleRef.internalName here, we want to make sure we have ts.jliMethodHandleRef in the "already loaded" classes later?

Do we save a significant amount of effort doing this compared to, e.g., listing these descriptors as vals somewhere and always loading the corresponding BTypes if the optimizer is on, without having to thread a "well-known BTypes" object throughout?

It probably doesn't matter here, but I tried to stick to the principle across the backend of not using strings for static internal names.

The background is: if a reference type p/C appears in generated code, ASM might invoke getCommonSuperClass for that type. At this point we need to be able to get to the corresponding ClassBType.

in this case I believe it's fine because if we get here the owner will have been loaded as a BType already. Added a comment.

lrytz · 2026-04-28T12:06:17Z

+
+  def srNothingRef(using Context): ClassBType =
+    if srNothingRefLazy eq null then
+      srNothingRefLazy = classBTypeFromSymbol(requiredClass("scala.runtime.Nothing$"))


There's no more lock on initializing srNothingRef and the other types here, is that fine? I think we should not have concurrent operations on the symbol table.

It might be theoretically possible for this not to be initialized before getting into the optimizer indeed...

The reason these three are not eagerly initialized, even though they'll basically always be needed, is that we have a test with just class java and that causes -Ycheck:all to fail if we load java.lang.Object because there are two things named java. I'll see if I can fix -Ycheck instead.

ok, loading the BTypes themselves isn't feasible because the class needs those fields as part of BType loading, but fetching the symbols from the symbol table at construction time is easy, so let's do that. it also avoids the problem from my previous comment.

… OptimizerSettings

…n TreeChecker

…thus thread safe val

…r does not capture a Context

…ts Context

…e paths

SolalPirelli marked this pull request as draft April 10, 2026 07:22

SolalPirelli force-pushed the solal/jvm-cleanup-more branch 2 times, most recently from 51479f4 to 9c5119c Compare April 10, 2026 12:33

SolalPirelli commented Apr 10, 2026

View reviewed changes

SolalPirelli force-pushed the solal/jvm-cleanup-more branch 2 times, most recently from bd6071a to e29a481 Compare April 13, 2026 11:04

SolalPirelli marked this pull request as ready for review April 13, 2026 12:33

SolalPirelli requested a review from lrytz April 13, 2026 12:33

SolalPirelli commented Apr 13, 2026

View reviewed changes

SolalPirelli commented Apr 15, 2026

View reviewed changes

SolalPirelli force-pushed the solal/jvm-cleanup-more branch 3 times, most recently from b5f6a77 to 91daf55 Compare April 23, 2026 13:01

lrytz reviewed Apr 28, 2026

View reviewed changes

SolalPirelli added 12 commits April 29, 2026 13:48

Remove PPFA.CompilerSettings, access settings directly or through new…

38ce2cc

… OptimizerSettings

Inject ClassPath into BCodeRepository directly

c7f8e80

Make PPFA final

b782d1e

Remove unnecessary refs to Phases.genBCodePhase

ee2df3d

Small simplification

320fbc9

Emit MODULE$ separately, avoiding the need for a genBCode reference i…

5eac605

…n TreeChecker

Minor cleanups to ease understanding/'find all references'

eb1b532

Remove the use of Context from GeneratedClassHandler

232cc5e

Dedupe FileWriters/ClassfileWriters

511dc16

Remove capture of Context by BackendUtils

3aef8cb

Remove PPFA from BackendUtils

28e32be

Remove needless indirection

ca7e5f4

SolalPirelli added 20 commits April 29, 2026 13:53

Encapsulate PostProcessor

3fa0b5f

Remove the need for AbstractFile-to-AbstractFile conversion

48fdbf9

Remove some locking around already concurrent structures

9a076eb

Remove unneeded deps from BCodeRepo

48e7848

Remove another unneeded lock around initializing an already lazy and …

2ef17b7

…thus thread safe val

Remove one more lock around a concurrent data structure

61f8321

Split CoreBTypes into separate BTypeLoader and WellKnownBTypes, forme…

08b5f90

…r does not capture a Context

Merge BCodeAsmCommon into BCodeUtils

029ce0c

Delete dead code

19602d5

Cleanup, avoid taking compilation unit as param when already taking i…

f3ff58f

…ts Context

Stop capturing Context in BCodeIdiomatic just for probably unreachabl…

41059a8

…e paths

Stop capturing Context in BCodeHelpers

1e56c63

Stop capturing Context in BCodeSkelBuilder

b09916b

Stop capturing Context in BCodeBodyBuilder

8c5e522

Stop capturing Context in BCodeSyncAndTry

4b7ae04

Stop capturing Context in CodeGen

99208e1

Stop editing the Context in GenBCode, clean up

bc07225

Finally we can remove this lock that looked unnecessary

1b45003

Missed one case of captured CompilationUnit

4112301

Stop explicitly referencing genBCode from Phases

258e041

SolalPirelli force-pushed the solal/jvm-cleanup-more branch from 91daf55 to 258e041 Compare April 29, 2026 11:53

PR feedback

1f41f56

SolalPirelli requested a review from lrytz April 29, 2026 14:44

                 }
-              }
+                /**

Conversation

SolalPirelli commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How much have you relied on LLM-based tools in this contribution?

How was the solution tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SolalPirelli Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SolalPirelli commented Apr 23, 2026

Uh oh!

lrytz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SolalPirelli commented Apr 9, 2026 •

edited

Loading

SolalPirelli Apr 15, 2026 •

edited

Loading