Skip to content

Remove uses of unsafeNulls from the JVM backend#26094

Merged
SolalPirelli merged 2 commits into
scala:mainfrom
dotty-staging:solal/jvm-safe-nulls
Jun 3, 2026
Merged

Remove uses of unsafeNulls from the JVM backend#26094
SolalPirelli merged 2 commits into
scala:mainfrom
dotty-staging:solal/jvm-safe-nulls

Conversation

@SolalPirelli
Copy link
Copy Markdown
Contributor

@SolalPirelli SolalPirelli commented May 19, 2026

No more unsafeNulls in the entire compiler!

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

Not at all

How was the solution tested?

Covered by existing tests (this is a refactoring)

@SolalPirelli SolalPirelli force-pushed the solal/jvm-safe-nulls branch from 7773e17 to d3f9744 Compare May 19, 2026 08:32
case asm.Type.LONG => LONG
case asm.Type.FLOAT => FLOAT
case asm.Type.DOUBLE => DOUBLE
case _ => null
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

turns out this case was hit right below, so I refactored this to not need to have nullness

var cnode: ClassNode1 = null
var thisName: String = null // the internal name of the class being emitted
private var cnode: ClassNode1 = uninitialized
private var thisName: String = uninitialized // the internal name of the class being emitted
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

unfortunately given the code shape removing the need for uninitialized would require a fair amount of work. But at least we can make those private so nobody accidentally observes them when they shouldn't.

TraceUtils.traceMethodIfRequested(mnode)

mnode = null
mnode = null.asInstanceOf[MethodNode1] // for GC
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(I'm assuming)

@@ -88,22 +82,23 @@ private[jvm] object GeneratedClassHandler {
private val processingUnits = ListBuffer.empty[CompilationUnitInPostProcess]

def process(unit: GeneratedCompilationUnit): Unit = {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this class had the only use of uninitialized outside of the BCode* stuff and it looked easy to remove so... bonus cleanup

@SolalPirelli SolalPirelli force-pushed the solal/jvm-safe-nulls branch from d3f9744 to 58a1c8a Compare May 19, 2026 08:47
@SolalPirelli SolalPirelli requested a review from sjrd May 19, 2026 09:43
@SolalPirelli SolalPirelli force-pushed the solal/jvm-safe-nulls branch from 58a1c8a to c186a23 Compare May 26, 2026 14:06
Copy link
Copy Markdown
Member

@noti0na1 noti0na1 left a comment

Choose a reason for hiding this comment

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

LGTM, just need a rebase.

@SolalPirelli SolalPirelli merged commit 306f181 into scala:main Jun 3, 2026
45 checks passed
@SolalPirelli SolalPirelli deleted the solal/jvm-safe-nulls branch June 3, 2026 14:03
@SolalPirelli SolalPirelli added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Jun 4, 2026
@SolalPirelli SolalPirelli added this to the 3.9.0 milestone Jun 4, 2026
@SolalPirelli
Copy link
Copy Markdown
Contributor Author

Nominating for a 3.9 backport because this will make it easier to backport other stuff in the future. But it's not strictly required.

@WojciechMazur WojciechMazur added backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" and removed backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants