Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression in type inference of match-defined partial function #6925

Closed
scabug opened this issue Jan 6, 2013 · 5 comments
Closed

regression in type inference of match-defined partial function #6925

scabug opened this issue Jan 6, 2013 · 5 comments
Assignees
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Jan 6, 2013

As of scala/scala@2848373 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)
@scabug
Copy link
Author

@scabug scabug commented Jan 6, 2013

Loading

@scabug
Copy link
Author

@scabug scabug commented Jan 6, 2013

@paulp said:
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

Loading

@scabug
Copy link
Author

@scabug scabug commented Jan 8, 2013

@paulp said:
Please take a look at my comments in #5189 before we throw any more code at the problem.

Loading

@scabug
Copy link
Author

@scabug scabug commented Jan 8, 2013

@adriaanm said:
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.

Loading

@scabug
Copy link
Author

@scabug scabug commented Jan 10, 2013

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants