Skip to content

Commit ecc6369

Browse files
committed
Merge pull request scala#4122 from retronym/ticket/7459-2
SI-7459 Handle pattern binders used as prefixes in TypeTrees.
2 parents 640c955 + 5b8327b commit ecc6369

17 files changed

+219
-13
lines changed

src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
266266
// the type of the binder passed to the first invocation
267267
// determines the type of the tree that'll be returned for that binder as of then
268268
final def binderToUniqueTree(b: Symbol) =
269-
unique(accumSubst(normalize(CODE.REF(b))), b.tpe)
269+
unique(accumSubst(normalize(gen.mkAttributedStableRef(b))), b.tpe)
270270

271271
// note that the sequencing of operations is important: must visit in same order as match execution
272272
// binderToUniqueTree uses the type of the first symbol that was encountered as the type for all future binders

src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,10 @@ trait MatchTranslation {
248248
if (caseDefs forall treeInfo.isCatchCase) caseDefs
249249
else {
250250
val swatches = { // switch-catches
251-
val bindersAndCases = caseDefs map { caseDef =>
251+
// SI-7459 must duplicate here as we haven't commited to switch emission, and just figuring out
252+
// if we can ends up mutating `caseDefs` down in the use of `substituteSymbols` in
253+
// `TypedSubstitution#Substitution`. That is called indirectly by `emitTypeSwitch`.
254+
val bindersAndCases = caseDefs.map(_.duplicate) map { caseDef =>
252255
// generate a fresh symbol for each case, hoping we'll end up emitting a type-switch (we don't have a global scrut there)
253256
// if we fail to emit a fine-grained switch, have to do translateCase again with a single scrutSym (TODO: uniformize substitution on treemakers so we can avoid this)
254257
val caseScrutSym = freshSym(pos, pureType(ThrowableTpe))
@@ -518,7 +521,7 @@ trait MatchTranslation {
518521
// reference the (i-1)th case accessor if it exists, otherwise the (i-1)th tuple component
519522
override protected def tupleSel(binder: Symbol)(i: Int): Tree = {
520523
val accessors = binder.caseFieldAccessors
521-
if (accessors isDefinedAt (i-1)) REF(binder) DOT accessors(i-1)
524+
if (accessors isDefinedAt (i-1)) gen.mkAttributedStableRef(binder) DOT accessors(i-1)
522525
else codegen.tupleSel(binder)(i) // this won't type check for case classes, as they do not inherit ProductN
523526
}
524527
}

src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,17 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
167167
val usedBinders = new mutable.HashSet[Symbol]()
168168
// all potentially stored subpat binders
169169
val potentiallyStoredBinders = stored.unzip._1.toSet
170+
def ref(sym: Symbol) =
171+
if (potentiallyStoredBinders(sym)) usedBinders += sym
170172
// compute intersection of all symbols in the tree `in` and all potentially stored subpat binders
171-
in.foreach(t => if (potentiallyStoredBinders(t.symbol)) usedBinders += t.symbol)
173+
in.foreach {
174+
case tt: TypeTree =>
175+
tt.tpe foreach { // SI-7459 e.g. case Prod(t) => new t.u.Foo
176+
case SingleType(_, sym) => ref(sym)
177+
case _ =>
178+
}
179+
case t => ref(t.symbol)
180+
}
172181

173182
if (usedBinders.isEmpty) in
174183
else {

src/compiler/scala/tools/nsc/transform/patmat/PatternMatching.scala

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import scala.language.postfixOps
1212
import scala.tools.nsc.transform.TypingTransformers
1313
import scala.tools.nsc.transform.Transform
1414
import scala.reflect.internal.util.Statistics
15-
import scala.reflect.internal.Types
15+
import scala.reflect.internal.{Mode, Types}
1616
import scala.reflect.internal.util.Position
1717

1818
/** Translate pattern matching.
@@ -198,33 +198,57 @@ trait Interface extends ast.TreeDSL {
198198
}
199199

200200
class Substitution(val from: List[Symbol], val to: List[Tree]) {
201-
import global.{Transformer, Ident, NoType}
201+
import global.{Transformer, Ident, NoType, TypeTree, SingleType}
202202

203203
// We must explicitly type the trees that we replace inside some other tree, since the latter may already have been typed,
204204
// and will thus not be retyped. This means we might end up with untyped subtrees inside bigger, typed trees.
205205
def apply(tree: Tree): Tree = {
206206
// according to -Ystatistics 10% of translateMatch's time is spent in this method...
207207
// since about half of the typedSubst's end up being no-ops, the check below shaves off 5% of the time spent in typedSubst
208-
if (!tree.exists { case i@Ident(_) => from contains i.symbol case _ => false}) tree
209-
else (new Transformer {
208+
val toIdents = to.forall(_.isInstanceOf[Ident])
209+
val containsSym = tree.exists {
210+
case i@Ident(_) => from contains i.symbol
211+
case tt: TypeTree => tt.tpe.exists {
212+
case SingleType(_, sym) =>
213+
(from contains sym) && {
214+
if (!toIdents) global.devWarning(s"Unexpected substitution of non-Ident into TypeTree `$tt`, subst= $this")
215+
true
216+
}
217+
case _ => false
218+
}
219+
case _ => false
220+
}
221+
val toSyms = to.map(_.symbol)
222+
object substIdentsForTrees extends Transformer {
210223
private def typedIfOrigTyped(to: Tree, origTp: Type): Tree =
211224
if (origTp == null || origTp == NoType) to
212225
// important: only type when actually substing and when original tree was typed
213226
// (don't need to use origTp as the expected type, though, and can't always do this anyway due to unknown type params stemming from polymorphic extractors)
214227
else typer.typed(to)
215228

229+
def typedStable(t: Tree) = typer.typed(t.shallowDuplicate, Mode.MonoQualifierModes | Mode.TYPEPATmode)
230+
lazy val toTypes: List[Type] = to map (tree => typedStable(tree).tpe)
231+
216232
override def transform(tree: Tree): Tree = {
217233
def subst(from: List[Symbol], to: List[Tree]): Tree =
218234
if (from.isEmpty) tree
219-
else if (tree.symbol == from.head) typedIfOrigTyped(to.head.shallowDuplicate.setPos(tree.pos), tree.tpe)
235+
else if (tree.symbol == from.head) typedIfOrigTyped(typedStable(to.head).setPos(tree.pos), tree.tpe)
220236
else subst(from.tail, to.tail)
221237

222-
tree match {
238+
val tree1 = tree match {
223239
case Ident(_) => subst(from, to)
224240
case _ => super.transform(tree)
225241
}
242+
tree1.modifyType(_.substituteTypes(from, toTypes))
226243
}
227-
}).transform(tree)
244+
}
245+
if (containsSym) {
246+
if (to.forall(_.isInstanceOf[Ident]))
247+
tree.duplicate.substituteSymbols(from, to.map(_.symbol)) // SI-7459 catches `case t => new t.Foo`
248+
else
249+
substIdentsForTrees.transform(tree)
250+
}
251+
else tree
228252
}
229253

230254

src/compiler/scala/tools/nsc/typechecker/Typers.scala

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3776,8 +3776,12 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
37763776
case TypeRef(pre, sym, args) =>
37773777
if (sym.isAliasType && containsLocal(tp) && (tp.dealias ne tp)) apply(tp.dealias)
37783778
else {
3779-
if (pre.isVolatile)
3780-
InferTypeWithVolatileTypeSelectionError(tree, pre)
3779+
if (pre.isVolatile) pre match {
3780+
case SingleType(_, sym) if sym.isSynthetic && isPastTyper =>
3781+
debuglog(s"ignoring volatility of prefix in pattern matcher generated inferred type: $tp") // See pos/t7459c.scala
3782+
case _ =>
3783+
InferTypeWithVolatileTypeSelectionError(tree, pre)
3784+
}
37813785
mapOver(tp)
37823786
}
37833787
case _ =>

test/files/pos/t7459a.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
trait SpecialException extends Throwable
2+
3+
object Test {
4+
def run() {
5+
try {
6+
???
7+
} catch {
8+
case e: SpecialException => e.isInstanceOf[SpecialException]
9+
case e =>
10+
}
11+
12+
// OKAY
13+
// (null: Throwable) match {
14+
// case e: SpecialException => e.isInstanceOf[SpecialException]
15+
// case e =>
16+
// }
17+
}
18+
}

test/files/pos/t7459b.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import scala.concurrent._
2+
import scala.util._
3+
4+
5+
class Test {
6+
(null: Any) match {
7+
case s @ Some(_) => ???
8+
case f @ _ =>
9+
() => f
10+
???
11+
}
12+
}

test/files/pos/t7459c.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
object Test {
2+
trait Universe {
3+
type Type
4+
type TypeTag[A] >: Null <: TypeTagApi[A]
5+
trait TypeTagApi[A] { def tpe: Type }
6+
}
7+
trait JavaUniverse extends Universe
8+
9+
trait Mirror[U <: Universe] {
10+
def universe: U
11+
}
12+
(null: Mirror[_]).universe match {
13+
case ju: JavaUniverse =>
14+
val ju1 = ju
15+
val f = {() => (null: ju.TypeTag[Nothing]).tpe }
16+
}
17+
trait M[A]
18+
}

test/files/pos/t7459d.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
class Test {
2+
(null: Any) match {
3+
case s @ Some(_) => ???
4+
case f @ _ =>
5+
() => f
6+
???
7+
}
8+
}

test/files/pos/t7704.scala

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
class Attr { type V ; class Val }
2+
class StrAttr extends Attr { type V = String }
3+
class BoolAttr extends Attr { type V = Boolean }
4+
5+
object Main {
6+
def f(x: Attr) = x match {
7+
case v: StrAttr => new v.Val
8+
case v: BoolAttr => new v.Val
9+
}
10+
}

test/files/run/t7459a.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
class LM {
2+
class Node[B1]
3+
case class CC(n: LM)
4+
5+
// crash
6+
val f: (LM => Any) = {
7+
case tttt =>
8+
new tttt.Node[Any]()
9+
}
10+
}
11+
12+
object Test extends App {
13+
new LM().f(new LM())
14+
}

test/files/run/t7459b-optimize.flags

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-optimize

test/files/run/t7459b-optimize.scala

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
class LM {
2+
class Node[B1]
3+
4+
// crash
5+
val g: (CC => Any) = {
6+
case CC(tttt) =>
7+
new tttt.Node[Any]()
8+
}
9+
10+
val h: (Some[CC] => Any) = {
11+
case Some(CC(tttt)) =>
12+
new tttt.Node[Any]()
13+
}
14+
}
15+
16+
object Test extends App {
17+
new LM().g(new CC(new LM()))
18+
new LM().h(Some(new CC(new LM())))
19+
}
20+
case class CC(n: LM)
21+

test/files/run/t7459b.scala

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
class LM {
2+
class Node[B1]
3+
4+
// crash
5+
val g: (CC => Any) = {
6+
case CC(tttt) =>
7+
new tttt.Node[Any]()
8+
}
9+
10+
val h: (Some[CC] => Any) = {
11+
case Some(CC(tttt)) =>
12+
new tttt.Node[Any]()
13+
}
14+
}
15+
16+
object Test extends App {
17+
new LM().g(new CC(new LM()))
18+
new LM().h(Some(new CC(new LM())))
19+
}
20+
case class CC(n: LM)
21+

test/files/run/t7459c.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
class LM {
2+
class Node[B1]
3+
4+
// crash
5+
val g: (CC => Any) = {
6+
case CC(tttt) =>
7+
tttt.## // no crash
8+
new tttt.Node[Any]()
9+
}
10+
}
11+
12+
object Test extends App {
13+
new LM().g(new CC(new LM()))
14+
}
15+
case class CC(n: LM)
16+

test/files/run/t7459d.scala

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
class LM {
2+
class Node[B1]
3+
case class CC(n: LM)
4+
5+
// crash
6+
val f: (LM => Any) = {
7+
case tttt =>
8+
val uuuu: (tttt.type, Any) = (tttt, 0)
9+
new uuuu._1.Node[Any]()
10+
}
11+
}
12+
13+
object Test extends App {
14+
new LM().f(new LM())
15+
}

test/files/run/t7459f.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
object Test extends App {
2+
class C
3+
4+
case class FooSeq(x: Int, y: String, z: C*)
5+
6+
FooSeq(1, "a", new C()) match {
7+
case FooSeq(1, "a", x@_* ) =>
8+
//println(x.toList)
9+
x.asInstanceOf[x.type]
10+
assert(x.isInstanceOf[x.type])
11+
}
12+
}

0 commit comments

Comments
 (0)