Skip to content

Commit d1a3261

Browse files
committed
Treat Scala.js pseudo-unions in Scala 2 symbols as real unions
When unpickling a Scala 2 symbol, we now treat `scala.scalajs.js.|[A, B]` as if it were a real `A | B`, this requires a special-case in erasure to emit the correct signatures in SJSIR. Remaining issues after this commit fixed in later commits of this PR: - The companion object of `js.|` defines implicit conversions like `undefOr2ops`, those are no longer automatically in scope for values with a union type (which breaks many JUnitTests). - When compiling Scala.js code from source, `A | B` is interpreted as `js.|[A, B]` if `js.|` is in scope (e.g. via an import). Additionally, a few tests had to be disabled until we can depend on their fixed upstream version (scala-js/scala-js#4451).
1 parent 7c6232e commit d1a3261

File tree

8 files changed

+79
-6
lines changed

8 files changed

+79
-6
lines changed

compiler/src/dotty/tools/dotc/core/TypeErasure.scala

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import Symbols._, Types._, Contexts._, Flags._, Names._, StdNames._, Phases._
66
import Flags.JavaDefined
77
import Uniques.unique
88
import TypeOps.makePackageObjPrefixExplicit
9+
import backend.sjs.JSDefinitions
910
import transform.ExplicitOuter._
1011
import transform.ValueClasses._
1112
import transform.TypeUtils._
@@ -523,7 +524,19 @@ class TypeErasure(sourceLanguage: SourceLanguage, semiEraseVCs: Boolean, isConst
523524
else
524525
erasedGlb(this(tp1), this(tp2), isJava = sourceLanguage.isJava)
525526
case OrType(tp1, tp2) =>
526-
TypeComparer.orType(this(tp1), this(tp2), isErased = true)
527+
if isSymbol && sourceLanguage.isScala2 && ctx.settings.scalajs.value then
528+
// In Scala2Unpickler we unpickle Scala.js pseudo-unions as if they were
529+
// real unions, but we must still erase them as Scala 2 would to emit
530+
// the correct signatures in SJSIR.
531+
// We only do this when `isSymbol` is true since in other situations we
532+
// cannot distinguish a Scala.js pseudo-union from a Scala 3 union that
533+
// has been substituted into a Scala 2 type (e.g., via `asSeenFrom`),
534+
// erasing these unions as if they were pseudo-unions could have an
535+
// impact on overriding relationships so it's best to leave them
536+
// alone (and this doesn't impact the SJSIR we generate).
537+
JSDefinitions.jsdefn.PseudoUnionType
538+
else
539+
TypeComparer.orType(this(tp1), this(tp2), isErased = true)
527540
case tp: MethodType =>
528541
def paramErasure(tpToErase: Type) =
529542
erasureFn(sourceLanguage, semiEraseVCs, isConstructor, isSymbol, wildcardOK)(tpToErase)

compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import NameKinds.{Scala2MethodNameKinds, SuperAccessorName, ExpandedName}
1313
import util.Spans._
1414
import dotty.tools.dotc.ast.{tpd, untpd}, ast.tpd._
1515
import ast.untpd.Modifiers
16+
import backend.sjs.JSDefinitions
1617
import printing.Texts._
1718
import printing.Printer
1819
import io.AbstractFile
@@ -675,6 +676,10 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
675676

676677
def removeSingleton(tp: Type): Type =
677678
if (tp isRef defn.SingletonClass) defn.AnyType else tp
679+
def mapArg(arg: Type) = arg match {
680+
case arg: TypeRef if isBound(arg) => arg.symbol.info
681+
case _ => arg
682+
}
678683
def elim(tp: Type): Type = tp match {
679684
case tp @ RefinedType(parent, name, rinfo) =>
680685
val parent1 = elim(tp.parent)
@@ -690,12 +695,11 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
690695
}
691696
case tp @ AppliedType(tycon, args) =>
692697
val tycon1 = tycon.safeDealias
693-
def mapArg(arg: Type) = arg match {
694-
case arg: TypeRef if isBound(arg) => arg.symbol.info
695-
case _ => arg
696-
}
697698
if (tycon1 ne tycon) elim(tycon1.appliedTo(args))
698699
else tp.derivedAppliedType(tycon, args.map(mapArg))
700+
case tp: AndOrType =>
701+
// scalajs.js.|.UnionOps has a type parameter upper-bounded by `_ | _`
702+
tp.derivedAndOrType(mapArg(tp.tp1).bounds.hi, mapArg(tp.tp2).bounds.hi)
699703
case _ =>
700704
tp
701705
}
@@ -777,6 +781,12 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
777781
val tycon = select(pre, sym)
778782
val args = until(end, () => readTypeRef())
779783
if (sym == defn.ByNameParamClass2x) ExprType(args.head)
784+
else if (ctx.settings.scalajs.value && args.length == 2 &&
785+
sym.owner == JSDefinitions.jsdefn.ScalaJSJSPackageClass && sym == JSDefinitions.jsdefn.PseudoUnionClass) {
786+
// Treat Scala.js pseudo-unions as real unions, this requires a
787+
// special-case in erasure, see TypeErasure#eraseInfo.
788+
OrType(args(0), args(1), soft = false)
789+
}
780790
else if (args.nonEmpty) tycon.safeAppliedTo(EtaExpandIfHK(sym.typeParams, args.map(translateTempPoly)))
781791
else if (sym.typeParams.nonEmpty) tycon.EtaExpand(sym.typeParams)
782792
else tycon

project/Build.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1105,9 +1105,12 @@ object Build {
11051105
-- "ObjectTest.scala" // compile errors caused by #9588
11061106
-- "StackTraceTest.scala" // would require `npm install source-map-support`
11071107
-- "UnionTypeTest.scala" // requires the Scala 2 macro defined in Typechecking*.scala
1108+
-- "PromiseMock.scala" -- "AsyncTest.scala" // TODO: Enable once we use a Scala.js with https://github.com/scala-js/scala-js/pull/4451 in
11081109
)).get
11091110

1110-
++ (dir / "js/src/test/require-2.12" ** "*.scala").get
1111+
++ (dir / "js/src/test/require-2.12" ** (("*.scala": FileFilter)
1112+
-- "JSOptionalTest212.scala" // TODO: Enable once we use a Scala.js with https://github.com/scala-js/scala-js/pull/4451 in
1113+
)).get
11111114
++ (dir / "js/src/test/require-sam" ** "*.scala").get
11121115
++ (dir / "js/src/test/scala-new-collections" ** "*.scala").get
11131116
)
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
lazy val scala2Lib = project.in(file("scala2Lib"))
2+
.enablePlugins(ScalaJSPlugin)
3+
.settings(
4+
// TODO: switch to 2.13.5 once we've upgrade sbt-scalajs to 1.5.0
5+
scalaVersion := "2.13.4"
6+
)
7+
8+
lazy val dottyApp = project.in(file("dottyApp"))
9+
.dependsOn(scala2Lib)
10+
.enablePlugins(ScalaJSPlugin)
11+
.settings(
12+
scalaVersion := sys.props("plugin.scalaVersion"),
13+
14+
scalaJSUseMainModuleInitializer := true,
15+
scalaJSLinkerConfig ~= (_.withCheckIR(true)),
16+
)
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
object Main {
2+
def main(args: Array[String]): Unit = {
3+
val a = new scala2Lib.A
4+
assert(a.foo(1) == "1")
5+
assert(a.foo("") == "1")
6+
assert(a.foo(Array(1)) == "2")
7+
8+
val b = new scala2Lib.B
9+
assert(b.foo(1) == "1")
10+
assert(b.foo("") == "1")
11+
assert(b.foo(Array(1)) == "2")
12+
}
13+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
addSbtPlugin("ch.epfl.lamp" % "sbt-dotty" % sys.props("plugin.version"))
2+
addSbtPlugin("org.scala-js" % "sbt-scalajs" % sys.props("plugin.scalaJSVersion"))
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Keep synchronized with dottyApp/Api.scala
2+
package scala2Lib
3+
4+
import scala.scalajs.js
5+
import js.|
6+
7+
class A {
8+
def foo(x: Int | String): String = "1"
9+
def foo(x: Array[Int]): String = "2"
10+
}
11+
12+
class B extends js.Object {
13+
def foo(x: Int | String): String = "1"
14+
def foo(x: Array[Int]): String = "2"
15+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
> dottyApp/run

0 commit comments

Comments
 (0)