From cad94ff3f475ec6620153601c65c466f833a9711 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 8 Feb 2021 11:11:36 +1000 Subject: [PATCH] More accurate outer checks in patterns 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. --- .../transform/patmat/MatchTreeMaking.scala | 87 ++++++++++--- test/files/neg/t7721.check | 20 ++- test/files/pos/t11534.scala | 8 ++ test/files/run/t11534b.scala | 24 ++++ test/files/run/t11534c.scala | 117 ++++++++++++++++++ 5 files changed, 238 insertions(+), 18 deletions(-) create mode 100644 test/files/pos/t11534.scala create mode 100644 test/files/run/t11534b.scala create mode 100644 test/files/run/t11534c.scala diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala index 414407141b26..37a391a01942 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala @@ -347,9 +347,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 = { @@ -361,20 +358,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(nme.OUTER_SYNTH, 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 .P a subtype of .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.upperBound.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(nme.OUTER_SYNTH, newFlags = SYNTHETIC | ARTIFACT) setInfo expectedPrefix + val outerTest = (Select(codegen._asInstanceOf(testedBinder, expectedTp), synthOuterGetter)) OBJ_EQ expectedOuterRef + and(orig, outerTest) + } } } } diff --git a/test/files/neg/t7721.check b/test/files/neg/t7721.check index 04ef4858356c..2fa50df39c8d 100644 --- a/test/files/neg/t7721.check +++ b/test/files/neg/t7721.check @@ -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 diff --git a/test/files/pos/t11534.scala b/test/files/pos/t11534.scala new file mode 100644 index 000000000000..bab4bd956d87 --- /dev/null +++ b/test/files/pos/t11534.scala @@ -0,0 +1,8 @@ +// scalac: -Werror +object Test1 { + val g: scala.tools.nsc.Global = ??? + import g._ + def test(sym: Symbol) = sym.name match { + case _: TermName => + } +} diff --git a/test/files/run/t11534b.scala b/test/files/run/t11534b.scala new file mode 100644 index 000000000000..75e835bed9a3 --- /dev/null +++ b/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) + } +} diff --git a/test/files/run/t11534c.scala b/test/files/run/t11534c.scala new file mode 100644 index 000000000000..4fb201c64b4d --- /dev/null +++ b/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)) + } +}