Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dealias val aliases to modules so they're =:= #9313

Merged
merged 1 commit into from Feb 3, 2021

Conversation

@dwijnand
Copy link
Member

@dwijnand dwijnand commented Nov 12, 2020

This changes the definition of =:= in the smallest possible way:
if you have a singleton type that widens to a module,
then widen to that module.

Fixes scala/bug#12186.

@scala-jenkins scala-jenkins added this to the 2.13.5 milestone Nov 12, 2020
@lrytz
Copy link
Member

@lrytz lrytz commented Nov 13, 2020

Here's an example. In 2.13.3:

scala> :power
scala> import reflect.internal.util.{shortClassOfInstance => c}

scala> object O; val x: O.type = O
object O
val x: O.type = O$@1545b9da

scala> val oSingle = typeOf[O.type]; println(c(oSingle))
UniqueSingleType
val oSingle: $r.intp.global.Type = O.type

scala> val oModule = oSingle.underlying; println(c(oModule))
ModuleTypeRef
val oModule: $r.intp.global.Type = O.type

scala> val xSingle = typeOf[x.type]; println(c(xSingle))
UniqueSingleType
val xSingle: $r.intp.global.Type = x.type

scala> xSingle =:= oSingle
val res3: Boolean = true

scala> xSingle =:= oModule
val res4: Boolean = false

This PR fixes the last line.

@retronym
Copy link
Member

@retronym retronym commented Nov 14, 2020

We should unit test this by adding some test cases to

def testTransitivityWithModuleTypeRef(): Unit = {

See the previous tweaks to normalizePlus that I test drive with that test: 6f8c057

@retronym
Copy link
Member

@retronym retronym commented Nov 16, 2020

e.g.

diff --git a/test/junit/scala/reflect/internal/TypesTest.scala b/test/junit/scala/reflect/internal/TypesTest.scala
index ab0cdd954e..fc30775b4a 100644
--- a/test/junit/scala/reflect/internal/TypesTest.scala
+++ b/test/junit/scala/reflect/internal/TypesTest.scala
@@ -46,9 +46,15 @@ class TypesTest {
     val tp1 = TypeRef(ThisType(EmptyPackageClass), moduleClass, Nil)
     val tp2 = SingleType(ThisType(EmptyPackageClass), module)
     val tp3 = ThisType(moduleClass)
-    val tps = List(tp1, tp2, tp3)
+
+    val (otherModule, otherModuleClass) = EmptyPackageClass.newModuleAndClassSymbol(TermName("Other"), NoPosition, 0L)
+    val aliasSym = otherModuleClass.newTermSymbol(TermName("alias")).setInfo(tp2)
+    val tp4 = singleType(TypeRef(ThisType(EmptyPackageClass), otherModuleClass, Nil), aliasSym)
+
+
+    val tps = List(tp1, tp2, tp3, tp4)
     val results = mutable.Buffer[String]()
-    tps.permutations.foreach {
+    tps.combinations(3).foreach {
       case ts @ List(a, b, c) =>
         def tsShownRaw = ts.map(t => showRaw(t)).mkString(", ")
         if (a <:< b && b <:< c && !(a <:< c)) results += s"<:< intransitive: $tsShownRaw"

@dwijnand dwijnand force-pushed the dealias-scala.Nil branch from ae5f764 to 2eeada6 Nov 16, 2020
@dwijnand
Copy link
Member Author

@dwijnand dwijnand commented Nov 16, 2020

Thanks, Jason. I was getting some detail there wrong so I was struggling. Yours works very well, failing without the patch.

@dwijnand dwijnand force-pushed the dealias-scala.Nil branch from 2eeada6 to de23b34 Nov 16, 2020
This changes the definitely of =:= in the smallest possible way:
if you have a singleton type that widens to a module,
then widen to that module.
@dwijnand dwijnand force-pushed the dealias-scala.Nil branch from de23b34 to 53fad3d Nov 17, 2020
@dwijnand dwijnand requested a review from retronym Dec 4, 2020
@dwijnand
Copy link
Member Author

@dwijnand dwijnand commented Feb 3, 2021

Briefly touched on this one with Adriaan and he said he agrees that scala.Nil.type shouldn't be different to scala.collection.immutable.Nil.type. Any chance we can merge this?

lrytz
lrytz approved these changes Feb 3, 2021
@lrytz lrytz merged commit d37e99a into scala:2.13.x Feb 3, 2021
3 checks passed
@dwijnand dwijnand deleted the dealias-scala.Nil branch Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants