Skip to content

Commit

Permalink
Don't GLB binders of type patterns, use the type directly
Browse files Browse the repository at this point in the history
In type patterns `c match { case x: T }`, the translation would assign
the GLB of `c`'s type and `T` to the varaible `x`.

This seems to trace back to the first version of the "virtual pattern
matcher". I could not find a similar use of `glb` in that revision
of the codebase. So I'm not sure if it was a new addition, or picked
up from the previous implementation.

https://github.com/scala/scala/blob/8a9fd64129926eea35f7dca181242855f14e153f/src/compiler/scala/tools/nsc/typechecker/PatMatVirtualiser.scala#L438-L440

In the test case, the GLB collapsed to `Null` because its
computation failed (combination of f-bounds, existentials, skolems that
I don't follow), see `throw GlbFailure`. This is how it's been for 14
years (b894f80). This resulted in a cast to `Null$` failing at
runtime.

I assume GLB is fixed in Scala 3, as this core of the type system has
a new implementation. But the test case as such doesn't compile in
Scala 3 due to the non-wildcard existential.
  • Loading branch information
lrytz committed Dec 22, 2022
1 parent 1045255 commit 84bf5ca
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 3 deletions.
Expand Up @@ -68,11 +68,10 @@ trait MatchTranslation {
def pos = tree.pos
def tpe = binder.info.dealias // the type of the variable bound to the pattern
def pt = unbound match {
case Star(tpt) => this glbWith seqType(tpt.tpe)
case Star(tpt) => seqType(tpt.tpe)
case TypeBound(tpe) => tpe
case tree => tree.tpe
}
def glbWith(other: Type) = glb(tpe :: other :: Nil).normalize

object SymbolAndTypeBound {
def unapply(tree: Tree): Option[(Symbol, Type)] = tree match {
Expand All @@ -94,7 +93,7 @@ trait MatchTranslation {

private def bindingStep(sub: Symbol, subpattern: Tree) = step(SubstOnlyTreeMaker(sub, binder))(rebindTo(subpattern))
private def equalityTestStep() = step(EqualityTestTreeMaker(binder, tree, pos))()
private def typeTestStep(sub: Symbol, subPt: Type) = step(TypeTestTreeMaker(sub, binder, subPt, glbWith(subPt))(pos))()
private def typeTestStep(sub: Symbol, subPt: Type) = step(TypeTestTreeMaker(sub, binder, subPt, subPt)(pos))()
private def alternativesStep(alts: List[Tree]) = step(AlternativesTreeMaker(binder, translatedAlts(alts), alts.head.pos))()
private def translatedAlts(alts: List[Tree]) = alts map (alt => rebindTo(alt).translate())
private def noStep() = step()()
Expand Down
2 changes: 2 additions & 0 deletions test/files/run/t12702.check
@@ -0,0 +1,2 @@
warning: 1 feature warning; re-run with -feature for details
IOS
17 changes: 17 additions & 0 deletions test/files/run/t12702.scala
@@ -0,0 +1,17 @@
object Test {
trait MFSS[X <: MFSS[_]]
trait CS extends MFSS[CS]
trait MFI { type ST }
case class MFSD[S](mFI: MFI {type ST = S})
case object IOS extends MFI { type ST = CS }
type SD = MFSD[S] forSome {
type S <: MFSS[S]
}
def bad(sd: SD) = sd.mFI match {
case ios: IOS.type => println(ios)
}
def main(args: Array[String]): Unit = {
val x = MFSD(IOS)
bad(x)
}
}
29 changes: 29 additions & 0 deletions test/junit/scala/tools/nsc/symtab/SymbolTableTest.scala
Expand Up @@ -49,4 +49,33 @@ class SymbolTableTest {
import symbolTable._
assertEquals(NoSymbol, NoSymbol.outerClass)
}

@Test def t12702_glb(): Unit = {
import symbolTable._
import SymbolTableTest.t12702._
val t1 = typeOf[IOS.type]
val t2 = {
val sd = typeOf[SD]
sd.memberType(sd.member(TermName("mFI"))).finalResultType
}
// t1: Test.IOS.type
// t2: Test.MFI{type +ST = S}

// Ends up in `throw GlbFailure` in glb => Null
assertTrue(definitions.NullTpe =:= glb(t1 :: t2 :: Nil))
}
}

object SymbolTableTest {
object t12702 {
import scala.language.existentials
trait MFSS[X <: MFSS[_]]
trait CS extends MFSS[CS]
trait MFI { type ST }
case class MFSD[S](mFI: MFI {type ST = S})
case object IOS extends MFI { type ST = CS }
type SD = MFSD[S] forSome {
type S <: MFSS[S]
}
}
}

0 comments on commit 84bf5ca

Please sign in to comment.