Do not simplify isInstanceOf if unrelated types might be subtypes at run-time by SolalPirelli · Pull Request #25535 · scala/scala3

SolalPirelli · 2026-03-16T13:01:19Z

Introduce typeMightBeSubtypeAtRuntime in Platform, and use it in TypeTestCasts to avoid simplifying type tests that may have different behavior at runtime on a specific platform.

Implement this method for the JVM, where everything is Serializable, and Scala.js, where integer semantics are a little funny due to JS's "everything is a double" model.

Drop the "a value type cannot be an instance of a reference type" error as it's wrong, 1.isInstanceOf[Object] is true, and move the neg test to a run test ensuring this.

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

not

How was the solution tested?

new automated tests

sjrd · 2026-03-20T08:48:25Z


-            if foundEffectiveClass.isPrimitiveValueClass && !testCls.isPrimitiveValueClass then
+            if foundEffectiveClass.isPrimitiveValueClass && !testCls.isPrimitiveValueClass && !ctx.platform.typeMightSubtypeAtRuntime(foundEffectiveClass, testCls) then
              report.error(em"cannot test if value of $exprType is a reference of $testCls", tree.srcPos)


This change does not seem right. The platform should not influence whether code compiles or not.

there's 1 Scala.js test failing because I commented this out, I'll figure it out on Monday

@sjrd I think this is a case of the compiler being a little over-eager with an error. This report could be a warning, there's nothing inherently broken with testing whether A is a subtype of B when we know it's unrelated. I pushed a commit with a comment explaining why we need to call the platform here.

Hum. IMO in that case we must either
a) emit the compile error on JS as well, or
b) make it platform-dependent warning.

It makes no sense to have a platform-dependent compile error (unless there are interop features involved).

a) seems the most conservative here. IMO it's not nonsensical. One can argue that it's just a language rule that we forbid prim.isInstanceOf[NonPrim]. It doesn't need to have anything to do with the platform behavior.

OK. Unfortunately the way we get here is not by a direct check in the code but by another part of this file doing some widening (what else? :) ) so let me figure this out...

@sjrd so it turns out a few lines above there already was the infra to do basically (b), the unreachable function, so I used it.

However it's not strictly a platform-dependent warning because if we're in a pattern match we instead error with "unreachable case". Why that's an error and not a warning I don't know, but I don't think this PR should change that.

SolalPirelli · 2026-03-24T14:34:56Z

  lazy val sjsJUnitTests = project.in(file("tests/sjs-junit")).
    enablePlugins(DottyJSPlugin).
-    dependsOn(`scala-library-sjs`).
+    dependsOn(`scala-library-sjs`, `scala3-compiler-bootstrapped`).


since these unit tests also depend on the compiler...

I'm afraid that's wrong. That puts the compiler code on the classpath of sjsJUnitTests ! (also fixing this should be a separate PR anyway)

SolalPirelli · 2026-03-24T14:35:27Z

+    dependsOn(`scala-library-sjs`, `scala3-compiler-bootstrapped`).
    settings(
      regularScalaJSProjectSettings,
      bspEnabled := false,


for the deleted file below, I changed it to a "warn" test since that aligns with how the compiler was handling other such issues

GitHub tip: you can comment on a whole file with the comment icon in the top right corner of a file's diff. This is true even for deleted files.

sjrd · 2026-03-25T10:49:34Z

  lazy val sjsJUnitTests = project.in(file("tests/sjs-junit")).
    enablePlugins(DottyJSPlugin).
-    dependsOn(`scala-library-sjs`).
+    dependsOn(`scala-library-sjs`, `scala3-compiler-bootstrapped`).


I'm afraid that's wrong. That puts the compiler code on the classpath of sjsJUnitTests ! (also fixing this should be a separate PR anyway)

sjrd · 2026-03-25T10:51:56Z

+-- Warning: tests/warn/typetest.scala:5:10 -----------------------------------------------------------------------------
+5 |  println(i.isInstanceOf[Object])                     // warning
+  |          ^
+  |          this will always yield false since type Int is a primitive and class Object is a reference


It's going to say true if you actually run it, though, isn't it?

If it's meant as a progression test, say so in a comment on the test.

no, it does say false, at least running on the JVM

Well, that would then be a bug, because (1: Any).isInstanceOf[Object] definitely answers true today. It would be inconsistent for 1.isInstanceOf[Object] to return false.

It makes sense to me thinking of : Any as a boxing operation, a value type instance isn't an instance of Object until you box it.

No. Boxing is supposed to be transparent in Scala. There's no "boxing operation".

For example, you can still reproduce the result true without ever upcasting to Any:

scala> def test[T](x: T): Boolean = x.isInstanceOf[Object]; test[Int](5) def test[T](x: T): Boolean val res90: Boolean = true

.. could you say in the PR description what this PR changes? I wanted to take a quick look to understand, having a summary would really help.

Will do as soon as we agree on what that is 😂

To be clear this test currently fails to compile on main: https://github.com/scala/scala3/blob/main/tests/neg/typetest.scala

It was more of a general remark; you know what you changed, the thoughts behind it, the research you did. It saves reviewers a lot of time if you can summarize it in the PR description and commit message.

sjrd · 2026-03-25T11:09:43Z

+    dependsOn(`scala-library-sjs`, `scala3-compiler-bootstrapped`).
    settings(
      regularScalaJSProjectSettings,
      bspEnabled := false,


GitHub tip: you can comment on a whole file with the comment icon in the top right corner of a file's diff. This is true even for deleted files.

SolalPirelli · 2026-03-26T08:40:39Z

@sjrd I pushed a silly commit that comments out this warning/error entirely and... the only test that fails in the compiler is this typetest.scala that appears to be wrong today too. Should I properly delete the warning/error and figure out how to make 1.isInstanceOf[Object] return true as it should?

sjrd · 2026-03-26T12:16:55Z

Let's do that, yes.

…run-time (scala#25535) Fixes scala#25037 Fixes scala#25097 Introduce `typeMightBeSubtypeAtRuntime` in `Platform`, and use it in `TypeTestCasts` to avoid simplifying type tests that may have different behavior at runtime on a specific platform. Implement this method for the JVM, where everything is `Serializable`, and Scala.js, where integer semantics are a little funny due to JS's "everything is a double" model. Drop the "a value type cannot be an instance of a reference type" error as it's wrong, `1.isInstanceOf[Object]` is `true`, and move the neg test to a run test ensuring this. ## How much have your relied on LLM-based tools in this contribution? not ## How was the solution tested? new automated tests [Cherry-picked e0dd739][modified]

SolalPirelli changed the title ~~Do not simplify isInstanceOf if unrelated types might be subtypes at runtime~~ Do not simplify isInstanceOf if unrelated types might be subtypes at run-time Mar 16, 2026

SolalPirelli requested a review from sjrd March 17, 2026 07:59

SolalPirelli assigned sjrd Mar 17, 2026

SolalPirelli marked this pull request as ready for review March 17, 2026 08:00

sjrd requested changes Mar 20, 2026

View reviewed changes

sjrd assigned SolalPirelli and unassigned sjrd Mar 20, 2026

SolalPirelli force-pushed the solal/runtime-instanceof branch from 447c0ef to f30fed2 Compare March 20, 2026 15:11

sjrd approved these changes Mar 20, 2026

View reviewed changes

SolalPirelli commented Mar 24, 2026

View reviewed changes

SolalPirelli force-pushed the solal/runtime-instanceof branch from 50cfe64 to 8f2b599 Compare March 25, 2026 08:00

SolalPirelli requested a review from sjrd March 25, 2026 10:34

sjrd requested changes Mar 25, 2026

View reviewed changes

SolalPirelli and others added 13 commits March 26, 2026 14:46

JVM fix

3891651

JS version

aabafcb

Fixes per CI

fce0ad0

PR feedback

ea9213f

Clarify

e049d44

is it this simple?

dbae29e

final?

06ee6c3

Typo

1a61433

undo build change

f8e69d9

Fix?

77154e9

test what happens if we just don't have this warn/error

d55e0ac

Just remove it

c7a77d6

ok

ed998a6

SolalPirelli force-pushed the solal/runtime-instanceof branch from fe36f98 to ed998a6 Compare March 26, 2026 13:55

SolalPirelli requested review from lrytz and sjrd March 26, 2026 15:32

sjrd approved these changes Mar 26, 2026

View reviewed changes

lrytz approved these changes Mar 26, 2026

View reviewed changes

SolalPirelli merged commit e0dd739 into scala:main Mar 26, 2026
64 checks passed

SolalPirelli deleted the solal/runtime-instanceof branch March 26, 2026 20:32

WojciechMazur added this to the 3.8.4 milestone Mar 31, 2026

tgodzik mentioned this pull request Apr 8, 2026

Backport "Do not simplify isInstanceOf if unrelated types might be subtypes at run-time" to 3.3 LTS scala/scala3-lts#882

Merged

Conversation

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

Uh oh!

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

How was the solution tested?

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!

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!

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!

SolalPirelli commented Mar 26, 2026

Uh oh!

sjrd commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SolalPirelli commented Mar 16, 2026 •

edited

Loading