Skip to content

Commit

Permalink
SI-8128 Fix regression in extractors returning existentials
Browse files Browse the repository at this point in the history
The advent of the named based pattern matcher brought with it
a change in the way we determine the type of the value in the
"match monad". We used to take the base type to `Option` or `Seq`
(guided by the method name in `unapply` vs `unapplySeq`), and
simply use the type argument.

Name-based patmat, instead, uses the result type of methods in the
type. For example, the element type of an Option-like extractor
result is given by the result type of the no-args `get` method.

This approach, however, swiftly runs aground when navigating the
existential atolls. Here's why:

    scala> class F[_]
    defined class F

    scala> val tp = typeOf[Some[F[X]] forSome { type X }]
    warning: there were 1 feature warning(s); re-run with -feature for details
    tp: $r.intp.global.Type = scala.this.Some[F[X]] forSome { type X }

    scala> tp.baseType(typeOf[Option[_]].typeSymbol).typeArgs.head
    res10: $r.intp.global.Type = F[X] forSome { type X }

    scala> tp.memberType(tp.member(nme.get)).finalResultType
    res11: $r.intp.global.Type = F[X]

`res10` corresponds to 2.10.x approach in `matchMonadResult`.
`res11` corresponds to the new approach in `resultOfMatchingMethod`.

The last result is not wrapped by the existential type. This results
in errors like (shown under -Ydebug to turn un accurate printing of
skolems):

    error: error during expansion of this match (this is a scalac bug).
    The underlying error was: type mismatch;
     found   : _$1&0 where type _$1&0
     required: _$1
      (0: Any) match {
               ^
    one error found

This commit addresses the regression in 2.10.x compatible extractors
by using the 2.10 approach for them.

The residual problem is shown in the enclosed pending test.
  • Loading branch information
retronym committed Jan 9, 2014
1 parent 969a269 commit 3e9e2c6
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 4 deletions.
15 changes: 11 additions & 4 deletions src/reflect/scala/reflect/internal/Definitions.scala
Expand Up @@ -819,11 +819,18 @@ trait Definitions extends api.StandardDefinitions {
// FYI the long clunky name is because it's really hard to put "get" into the
// name of a method without it sounding like the method "get"s something, whereas
// this method is about a type member which just happens to be named get.
def typeOfMemberNamedGet(tp: Type) = resultOfMatchingMethod(tp, nme.get)()
def typeOfMemberNamedHead(tp: Type) = resultOfMatchingMethod(tp, nme.head)()
def typeOfMemberNamedApply(tp: Type) = resultOfMatchingMethod(tp, nme.apply)(IntTpe)
def typeOfMemberNamedDrop(tp: Type) = resultOfMatchingMethod(tp, nme.drop)(IntTpe)
def typeOfMemberNamedGet(tp: Type) = typeArgOfBaseTypeOr(tp, OptionClass)(resultOfMatchingMethod(tp, nme.get)())
def typeOfMemberNamedHead(tp: Type) = typeArgOfBaseTypeOr(tp, SeqClass)(resultOfMatchingMethod(tp, nme.head)())
def typeOfMemberNamedApply(tp: Type) = typeArgOfBaseTypeOr(tp, SeqClass)(resultOfMatchingMethod(tp, nme.apply)(IntTpe))
def typeOfMemberNamedDrop(tp: Type) = typeArgOfBaseTypeOr(tp, SeqClass)(resultOfMatchingMethod(tp, nme.drop)(IntTpe))
def typesOfSelectors(tp: Type) = getterMemberTypes(tp, productSelectors(tp))
// SI-8128 Still using the type argument of the base type at Seq/Option if this is an old-style (2.10 compatible)
// extractor to limit exposure to regressions like the reported problem with existentials.
// TODO fix the existential problem in the general case, see test/pending/pos/t8128.scala
private def typeArgOfBaseTypeOr(tp: Type, baseClass: Symbol)(or: => Type): Type = (tp baseType baseClass).typeArgs match {
case x :: Nil => x
case _ => or
}

// Can't only check for _1 thanks to pos/t796.
def hasSelectors(tp: Type) = (
Expand Down
15 changes: 15 additions & 0 deletions test/files/pos/t8128.scala
@@ -0,0 +1,15 @@
object G {
def unapply(m: Any): Option[_] = Some("")
}

object H {
def unapplySeq(m: Any): Option[Seq[_]] = None
}

object Test {
(0: Any) match {
case G(v) => v
case H(v) => v
case _ =>
}
}
18 changes: 18 additions & 0 deletions test/pending/pos/t8128b.scala
@@ -0,0 +1,18 @@
class Optiony[X] { def isEmpty = true; def get: X = ??? }
class Seqy[X] { def head: X = ???; def length = 0; def apply(i: Int): X = ??? }

object G {
def unapply(m: Any): Optiony[_] = ???
}

object H {
def unapplySeq(m: Any): Optiony[Seqy[_]] = ???
}

object Test {
(0: Any) match {
case G(v) => v
case H(v) => v
case _ =>
}
}

0 comments on commit 3e9e2c6

Please sign in to comment.