Skip to content

Commit

Permalink
[fport] More accurate outer checks in patterns
Browse files Browse the repository at this point in the history
Avoids eliding outer checks that matter (run/t11534b.scala) and
avoids emitting checks that don't (pos/t11534.scala) which avoids
compiler warnings when the tested class doesn't have an outer
field.

The latter stops the annoying unchecked warning that appeared since
a recent refactoring made `TermName` a final class.

(cherry picked from commit 1115959)
  • Loading branch information
retronym authored and dwijnand committed Feb 17, 2021
1 parent b74ea55 commit 5fff7dc
Show file tree
Hide file tree
Showing 5 changed files with 237 additions and 18 deletions.
87 changes: 70 additions & 17 deletions src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
Expand Up @@ -369,9 +369,6 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
def eqTest(pat: Tree, testedBinder: Symbol) = REF(testedBinder) OBJ_EQ pat

override def withOuterTest(orig: Tree)(testedBinder: Symbol, expectedTp: Type): Tree = {
val expectedPrefix = expectedTp.prefix
val testedPrefix = testedBinder.info.prefix

// Check if a type is defined in a static location. Unlike `tp.isStatic` before `flatten`,
// this also includes methods and (possibly nested) objects inside of methods.
def definedInStaticLocation(tp: Type): Boolean = {
Expand All @@ -383,20 +380,76 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
tp.typeSymbol.owner == tp.prefix.typeSymbol && isStatic(tp.prefix)
}

if ((expectedPrefix eq NoPrefix)
|| expectedTp.typeSymbol.isJava
|| definedInStaticLocation(expectedTp)
|| testedPrefix =:= expectedPrefix) orig
else gen.mkAttributedQualifierIfPossible(expectedPrefix) match {
case None => orig
case Some(expectedOuterRef) =>
// ExplicitOuter replaces `Select(q, outerSym) OBJ_EQ expectedPrefix`
// by `Select(q, outerAccessor(outerSym.owner)) OBJ_EQ expectedPrefix`
// if there's an outer accessor, otherwise the condition becomes `true`
// TODO: centralize logic whether there's an outer accessor and use here?
val synthOuterGetter = expectedTp.typeSymbol.newMethod(vpmName.outer, newFlags = SYNTHETIC | ARTIFACT) setInfo expectedPrefix
val outerTest = (Select(codegen._asInstanceOf(testedBinder, expectedTp), synthOuterGetter)) OBJ_EQ expectedOuterRef
and(orig, outerTest)
// In `def foo(a: b.B) match { case _: p.P }`
// testedBinder.symbol.info = b.B
// expectedTp = p.P

expectedTp.dealias match {
case RefinedType(Nil, _) => orig
case rt@RefinedType(parent :: rest, scope) =>
// If the pattern type is refined type, emit outer tests for each component.
withOuterTest(withOuterTest(orig)(testedBinder, parent))(testedBinder, copyRefinedType(rt, rest, scope))
case expectedTp =>
val expectedClass = expectedTp.typeSymbol
assert(!expectedClass.isRefinementClass, orig)
// .typeSymbol dealiases, so look at the prefix of the base type at the dealiased symbol,
// not of expectedTp itself.
val expectedPrefix = expectedTp.baseType(expectedClass).prefix

// If P is a subclass of B, and b =:= p, b.B
// If <fresh>.P a subtype of <fresh>.B and does b =:= p,
// we can assume that a value inhabiting _#P with b.P conforms to p.P.
//
// It is not sufficient to show that p.P is a subtype of p.B, as this
// would incorrectly elide the outer test in:
//
// class P extends p1.B
// def test(b: p1.B) = b match { case _: p1.P }
// test(new p2.P)
def prefixAligns: Boolean = {
expectedTp match {
case TypeRef(pre, _, _) if !pre.isStable => // e.g. _: Outer#Inner
false
case TypeRef(pre, sym, args) =>
val testedBinderClass = testedBinder.info.typeSymbol
val testedBinderType = testedBinder.info.baseType(testedBinderClass)

val testedPrefixIsExpectedTypePrefix = pre =:= testedBinderType.prefix
val testedPrefixAndExpectedPrefixAreStaticallyIdentical: Boolean = {
val freshPrefix = pre match {
case ThisType(thissym) =>
ThisType(thissym.cloneSymbol(thissym.owner))
case _ =>
val preSym = pre.termSymbol
val freshPreSym = preSym.cloneSymbol(preSym.owner).setInfo(preSym.info)
singleType(pre.prefix, freshPreSym)
}
val expectedTpFromFreshPrefix = TypeRef(freshPrefix, sym, args)
val baseTypeFromFreshPrefix = expectedTpFromFreshPrefix.baseType(testedBinderClass)
freshPrefix eq baseTypeFromFreshPrefix.prefix
}
testedPrefixAndExpectedPrefixAreStaticallyIdentical && testedPrefixIsExpectedTypePrefix
case _ =>
false
}
}

if ((expectedPrefix eq NoPrefix)
|| expectedTp.typeSymbol.isJava
|| definedInStaticLocation(expectedTp)
|| testedBinder.info <:< expectedTp
|| prefixAligns) orig
else gen.mkAttributedQualifierIfPossible(expectedPrefix) match {
case None => orig
case Some(expectedOuterRef) =>
// ExplicitOuter replaces `Select(q, outerSym) OBJ_EQ expectedPrefix`
// by `Select(q, outerAccessor(outerSym.owner)) OBJ_EQ expectedPrefix`
// if there's an outer accessor, otherwise the condition becomes `true`
// TODO: centralize logic whether there's an outer accessor and use here?
val synthOuterGetter = expectedTp.typeSymbol.newMethod(vpmName.outer, newFlags = SYNTHETIC | ARTIFACT) setInfo expectedPrefix
val outerTest = (Select(codegen._asInstanceOf(testedBinder, expectedTp), synthOuterGetter)) OBJ_EQ expectedOuterRef
and(orig, outerTest)
}
}
}
}
Expand Down
20 changes: 19 additions & 1 deletion test/files/neg/t7721.check
Expand Up @@ -22,6 +22,24 @@ t7721.scala:49: warning: abstract type pattern B.this.Foo is unchecked since it
t7721.scala:49: warning: abstract type pattern B.this.Bar is unchecked since it is eliminated by erasure
case x: Foo with Bar with Concrete => x.bippy + x.barry + x.dingo + x.conco + x.bongo
^
t7721.scala:13: warning: The outer reference in this type test cannot be checked at run time.
case x: Foo with Concrete => x.bippy + x.conco
^
t7721.scala:17: warning: The outer reference in this type test cannot be checked at run time.
case x: Concrete with Foo => x.bippy + x.conco
^
t7721.scala:21: warning: The outer reference in this type test cannot be checked at run time.
case x: Foo with Bar => x.bippy + x.barry
^
t7721.scala:41: warning: The outer reference in this type test cannot be checked at run time.
case x: Foo with Concrete => x.bippy + x.dingo + x.conco
^
t7721.scala:45: warning: The outer reference in this type test cannot be checked at run time.
case x: Concrete with Foo => x.bippy + x.dingo + x.conco
^
t7721.scala:49: warning: The outer reference in this type test cannot be checked at run time.
case x: Foo with Bar with Concrete => x.bippy + x.barry + x.dingo + x.conco + x.bongo
^
error: No warnings can be incurred under -Werror.
8 warnings
14 warnings
1 error
7 changes: 7 additions & 0 deletions test/files/pos/t11534.scala
@@ -0,0 +1,7 @@
object Test1 {
val g: scala.tools.nsc.Global = ???
import g._
def test(sym: Symbol) = sym.name match {
case _: TermName =>
}
}
24 changes: 24 additions & 0 deletions test/files/run/t11534b.scala
@@ -0,0 +1,24 @@
object Test {
case class O(i: Int) {
class A
class B extends A {
def bOuter = O.this
}
trait C {
def cOuter = O.this
}
class D extends o2.B with C
}
val o1 = new O(1);
val o2 = new O(2);
def pat1(a: Test.o1.C) = a match {
case b: Test.o1.B =>
assert(b.bOuter eq Test.o1,
s"expected ${o1} as outer of value conforming to pattern `b: Test.o1.B`, but got ${b.bOuter}")
case _ =>

}
def main(args: Array[String]): Unit = {
pat1(new o1.D)
}
}
117 changes: 117 additions & 0 deletions test/files/run/t11534c.scala
@@ -0,0 +1,117 @@
// scalac: -unchecked
import scala.util.Try

object Test {
class O(val i: Int) {
class A {
val aOuter = i
}

class B1 extends A {
val b1Outer = i
}
}
class M(i: Int) extends O(i) {
class B2 extends m2.A {
val b2Outer = i
}

def pat1(a: M.this.A) = a match {
case b: M.this.B1 => // can elide outer check, (a : m1.A) && (a : O#B1) implies (a : m1.B1)
assertOuter(m1.i, b.b1Outer)
true
case _ =>
false
}
def pat2(a: m2.A) = a match {
case b: M.this.B2 => // needs runtime outer check
assertOuter(m1.i, b.b2Outer)
true
case _ =>
false
}
def pat3(a: M.this.B1) = a match {
case b: M.this.A => // can elide outer check, (a : m1.B1) && (a : O#A) implies (a : m1.B1)
assertOuter(m1.i, b.aOuter)
true
case _ =>
false
}
def pat4(a: M.this.B2) = a match {
case b: m2.A => // can elide outer check, (a : m1.B2) implies (a : m2.A)
assertOuter(m2.i, b.aOuter)
true
case _ =>
false
}
}

val m1 = new M(1);
val m2 = new M(2);

def pat1(a: m1.A) = a match {
case b: m1.B1 => // can elide outer check, (a : m1.A) && (a : O#B1) implies (a : m1.B1)
assertOuter(m1.i, b.b1Outer)
true
case _ =>
false
}
def pat2(a: m2.A) = a match {
case b: m1.B2 => // needs runtime outer check
assertOuter(m1.i, b.b2Outer)
true
case _ =>
false
}
def pat3(a: m1.B1) = a match {
case b: m1.A => // can elide outer check, (a : m1.B1) && (a : O#A) implies (a : m1.B1)
assertOuter(m1.i, b.aOuter)
true
case _ =>
false
}
def pat4(a: m1.B2) = a match {
case b: m2.A => // can elide outer check, (a : m1.B2) implies (a : m2.A)
assertOuter(m2.i, b.aOuter)
true
case _ =>
false
}

def pat5(a: M#B2) = a match {
case b: m2.A => // can elide outer check, (a : A#B2) implies (a : m2.A)
assertOuter(m2.i, b.aOuter)
true
case _ =>
false
}
def assertOuter(expected: Int, actual: Int): Unit = {
if (expected != actual) throw WrongOuter(expected, actual)
}
case class WrongOuter(expected: Int, actual: Int) extends RuntimeException(s"expected: $expected, actual: $actual")

def main(args: Array[String]): Unit = {
assert(pat1(new m1.B1))
assert(m1.pat1(new m1.B1))
assert(Try(pat1((new m2.B1).asInstanceOf[m1.B1])).failed.get == WrongOuter(m1.i, m2.i))
assert(Try(m1.pat1((new m2.B1).asInstanceOf[m1.B1])).failed.get == WrongOuter(m1.i, m2.i))

assert(!pat2(new m2.B2))
assert(!m1.pat2(new m2.B2))
assert(pat2(new m1.B2))
assert(m1.pat2(new m1.B2))

assert(pat3(new m1.B1))
assert(m1.pat3(new m1.B1))
assert(Try(pat3((new m2.B1).asInstanceOf[m1.B1])).failed.get == WrongOuter(m1.i, m2.i))
assert(Try(m1.pat3((new m2.B1).asInstanceOf[m1.B1])).failed.get == WrongOuter(m1.i, m2.i))

assert(pat4(new m1.B2))
assert(m1.pat4(new m1.B2))
assert(pat4((new m2.B2).asInstanceOf[m1.B2]))
assert(m1.pat4((new m2.B2).asInstanceOf[m1.B2]))

assert(pat5(new m1.B2))
assert(pat5(new m2.B2))
}
}

0 comments on commit 5fff7dc

Please sign in to comment.