Permalink
Browse files

SI-7214 outer check based on dealiased pattern type.

A Typed Pattern (_: T) is more than `.isInstanceOf`: if `T` is a
path dependent type, the scrutinee's $outer reference is also
compared against the prefix of `T`.

The code that synthesises this is split into two places.
`needsOuterCheck` determines whether to add this check, based on
the type `T`, and the type of the scrutinee. If it gives the
go-ahead, `treeCondStrategy.outerCheck` synthesizes the check.

The new test case demonstrates the problems caused by the failure
to dealias in `needsOuterCheck`: it could either wrongly lead to
synthesis of an outer test (which would crash), or wrongly omit
the outer test (meaning overly liberal matching.)

A simple `dealias` remedies this. `dealiasWiden` is *not*
appropriate here; we need to keep hold of singleton types.

I'll also note that there is already a little slack between these
methods, as commented:

> 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: can we improve needsOuterTest so there's always an outerAccessor?

So this is probably a fragile area that warrants a careful review
with a view to design improvements.
  • Loading branch information...
1 parent b98cc58 commit acd74cae0935d79cedbeb2bec719174b7cf54e5e @retronym retronym committed Mar 5, 2013
Showing with 60 additions and 1 deletion.
  1. +3 −1 src/reflect/scala/reflect/internal/Types.scala
  2. +57 −0 test/files/run/t7214.scala
@@ -5354,7 +5354,9 @@ trait Types extends api.Types { self: SymbolTable =>
case _ =>
NoType
}
- patType match {
+ // See the test for SI-7214 for motivation for dealias. Later `treeCondStrategy#outerTest`
+ // generates an outer test based on `patType.prefix` with automatically dealises.
+ patType.dealias match {
case TypeRef(pre, sym, args) =>
val pre1 = maybeCreateDummyClone(pre, sym)
(pre1 ne NoType) && isPopulated(copyTypeRef(patType, pre1, sym, args), selType)
View
@@ -0,0 +1,57 @@
+// pattern matcher crashes here trying to synthesize an uneeded outer test.
+// no-symbol does not have an owner
+// at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:49)
+// at scala.tools.nsc.Global.abort(Global.scala:253)
+// at scala.reflect.internal.Symbols$NoSymbol.owner(Symbols.scala:3248)
+// at scala.reflect.internal.Symbols$Symbol.effectiveOwner(Symbols.scala:678)
+// at scala.reflect.internal.Symbols$Symbol.isDefinedInPackage(Symbols.scala:664)
+// at scala.reflect.internal.TreeGen.mkAttributedSelect(TreeGen.scala:188)
+// at scala.reflect.internal.TreeGen.mkAttributedRef(TreeGen.scala:124)
+// at scala.tools.nsc.ast.TreeDSL$CODE$.REF(TreeDSL.scala:308)
+// at scala.tools.nsc.typechecker.PatternMatching$TreeMakers$TypeTestTreeMaker$treeCondStrategy$.outerTest(PatternMatching.scala:1209)
+class Crash {
+ type Alias = C#T
+
+ val c = new C
+ val t = new c.T
+
+ // Crash via a Typed Pattern...
+ (t: Any) match {
+ case e: Alias =>
+ }
+
+ // ... or via a Typed Extractor Pattern.
+ object Extractor {
+ def unapply(a: Alias): Option[Any] = None
+ }
+ (t: Any) match {
+ case Extractor() =>
+ case _ =>
+ }
+
+ // checking that correct outer tests are applied when
+ // aliases for path dependent types are involved.
+ val c2 = new C
+ type CdotT = c.T
+ type C2dotT = c2.T
+
+ val outerField = t.getClass.getDeclaredFields.find(_.getName contains ("outer")).get
+ outerField.setAccessible(true)
+
+ (t: Any) match {
+ case _: C2dotT =>
+ println(s"!!! wrong match. t.outer=${outerField.get(t)} / c2 = $c2") // this matches on 2.10.0
+ case _: CdotT =>
+ case _ =>
+ println(s"!!! wrong match. t.outer=${outerField.get(t)} / c = $c")
+ }
+}
+
+class C {
+ class T
+}
+
+object Test extends App {
+ new Crash
+}
+

0 comments on commit acd74ca

Please sign in to comment.