Scala Programming Language
  1. Scala Programming Language
  2. SI-6925

regression in type inference of match-defined partial function

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: Scala 2.10.1
    • Component/s: None
    • Labels:
      None

      Description

      As of https://github.com/scala/scala/commit/28483739c3 this infers different types with and without -Xoldpatmat:

      % scala210
      Welcome to Scala version 2.10.0 (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_10).
      Type in expressions to have them evaluated.
      Type :help for more information.
      
      scala> def f[T](xs: Set[T]) = xs collect { case x => x }
      f: [T](xs: Set[T])scala.collection.immutable.Set[_ <: T]
      
      % scala210 -Xoldpatmat
      Welcome to Scala version 2.10.0 (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_10).
      Type in expressions to have them evaluated.
      Type :help for more information.
      
      scala> def f[T](xs: Set[T]) = xs collect { case x => x }
      f: [T](xs: Set[T])scala.collection.immutable.Set[T]
      

      It's a large change not to be mentioned in the commit message - is it unintentional? I assume it's not intentional that it's in one pattern matcher and not the other.

      commit 28483739c3
      Author: Adriaan Moors <adriaan.moors@epfl.ch>
      Date: 9 months ago

      restore typedMatchAnonFun in all its glory

      detect partialfunction in cpsannotationchecker
      emit apply/isDefinedAt if PF has @cps targs (applyOrElse can't be typed)
      further hacky improvements to selective anf
      better try/catch support in selective cps using freshly minted anonfun match
      make virtpatmat resilient to scaladoc (after uncurry, don't translate matches
      TODO: factor out translation all together so presentation compiler/scaladoc can skip it)

        Issue Links

          Activity

          Hide
          Paul Phillips added a comment -

          Here's an example of the consequences - this works in 2.9.2, but in 2.10:

          scala> List(1).flatMap(n => Set(1).collect { case w => w })
          <console>:8: error: no type parameters for method flatMap: (f: Int => scala.collection.GenTraversableOnce[B])(implicit bf: scala.collection.generic.CanBuildFrom[List[Int],B,That])That exist so that it can be applied to arguments (Int => scala.collection.immutable.Set[_ <: Int])
           --- because ---
          argument expression's type is not compatible with formal parameter type;
           found   : Int => scala.collection.immutable.Set[_ <: Int]
           required: Int => scala.collection.GenTraversableOnce[?B]
                        List(1).flatMap(n => Set(1).collect { case w => w })
                                ^
          <console>:8: error: type mismatch;
           found   : Int => scala.collection.immutable.Set[_ <: Int]
           required: Int => scala.collection.GenTraversableOnce[B]
                        List(1).flatMap(n => Set(1).collect { case w => w })
                                          ^
          <console>:8: error: Cannot construct a collection of type That with elements of type B based on a collection of type List[Int].
                        List(1).flatMap(n => Set(1).collect { case w => w })
                                       ^
          

          Mined from stackoverflow: http://stackoverflow.com/questions/14174639/flatmap-behavior-changed-in-2-10-0

          Show
          Paul Phillips added a comment - Here's an example of the consequences - this works in 2.9.2, but in 2.10: scala> List(1).flatMap(n => Set(1).collect { case w => w }) <console>:8: error: no type parameters for method flatMap: (f: Int => scala.collection.GenTraversableOnce[B])(implicit bf: scala.collection.generic.CanBuildFrom[List[Int],B,That])That exist so that it can be applied to arguments (Int => scala.collection.immutable.Set[_ <: Int]) --- because --- argument expression's type is not compatible with formal parameter type; found : Int => scala.collection.immutable.Set[_ <: Int] required: Int => scala.collection.GenTraversableOnce[?B] List(1).flatMap(n => Set(1).collect { case w => w }) ^ <console>:8: error: type mismatch; found : Int => scala.collection.immutable.Set[_ <: Int] required: Int => scala.collection.GenTraversableOnce[B] List(1).flatMap(n => Set(1).collect { case w => w }) ^ <console>:8: error: Cannot construct a collection of type That with elements of type B based on a collection of type List[Int]. List(1).flatMap(n => Set(1).collect { case w => w }) ^ Mined from stackoverflow: http://stackoverflow.com/questions/14174639/flatmap-behavior-changed-in-2-10-0
          Hide
          Paul Phillips added a comment -

          Please take a look at my comments in SI-5189 before we throw any more code at the problem.

          Show
          Paul Phillips added a comment - Please take a look at my comments in SI-5189 before we throw any more code at the problem.
          Hide
          Adriaan Moors added a comment -

          also wanted to quote Jason's (more general) comments from scala-internals on the matter:

          I was thinking about the way we synthesize the pattern matching partial functions some more, and was struck by (thankfully unjustified) fear about what happens in:

          object X { val pf: PartialFunction[Any, Any] =

          { case _ => this }

          }

          Serendipitously, the chicken/egg problem prevents us unhygenically binding `this` to the partial function:

          // should use the DefDef for the context's tree, but it doesn't exist yet (we need the typer we're creating to create it)
          val methodBodyTyper = newTyper(context.makeNewScope(context.tree, methodSym))
          ...
          val match_ = methodBodyTyper.typedMatch(gen.mkUnchecked(selector), cases, mode, ptRes)

          The This node carries the symbol for `Test`, even after the anon class is lifted out, which works out fine but was prompted me to double take before I'd reached for -Xprint-types.

          // cases.head.body.expr
          // this

          {Test.type}.=={(x$1: AnyRef)Boolean}(Test{Test.type}

          )

          {Boolean}

          Any local classes/methods defined in the case body do have correct parentage: the second argument to `makeNewScope` provides the method symbol, which is ready for children.

          Show
          Adriaan Moors added a comment - also wanted to quote Jason's (more general) comments from scala-internals on the matter: I was thinking about the way we synthesize the pattern matching partial functions some more, and was struck by (thankfully unjustified) fear about what happens in: object X { val pf: PartialFunction [Any, Any] = { case _ => this } } Serendipitously, the chicken/egg problem prevents us unhygenically binding `this` to the partial function: // should use the DefDef for the context's tree, but it doesn't exist yet (we need the typer we're creating to create it) val methodBodyTyper = newTyper(context.makeNewScope(context.tree, methodSym)) ... val match_ = methodBodyTyper.typedMatch(gen.mkUnchecked(selector), cases, mode, ptRes) The This node carries the symbol for `Test`, even after the anon class is lifted out, which works out fine but was prompted me to double take before I'd reached for -Xprint-types. // cases.head.body.expr // this {Test.type}.=={(x$1: AnyRef)Boolean}(Test{Test.type} ) {Boolean} Any local classes/methods defined in the case body do have correct parentage: the second argument to `makeNewScope` provides the method symbol, which is ready for children.
          Show
          Adriaan Moors added a comment - https://github.com/scala/scala/pull/1878

            People

            • Assignee:
              Adriaan Moors
              Reporter:
              Paul Phillips
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development