Skip to content

Compiler generated implicit conversion between function literal and PartialFunction #4241

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

Closed
allanrenucci opened this issue Apr 4, 2018 · 3 comments

Comments

@allanrenucci
Copy link
Contributor

allanrenucci commented Apr 4, 2018

Scalac and Dotty both implicitly convert from function literals to PartialFunction. For example, one can write:

val foo: PartialFunction[Int, String] = x => x.toString

However this conversion is different in Dotty and Scalac. The compilers need to generate an implementation for def isDefinedAt(x: A): Boolean. Both compilers inspect the rhs of the function literal:

  • Dotty: if the rhs is a match, implements isDefinedAt in term of the cases, otherwise returns true
  • Scalac: if the rhs is a match, implements isDefinedAt in term of the cases, otherwise fails to compile

This leads to surprising behaviors:

val a: PartialFunction[Int, String] = x => x match { case 1 => x.toString }
a.isDefinedAt(2) // Dotty: false, Scalac: false

val b: PartialFunction[Int, String] = x => { x match { case 1 => x.toString } }
b.isDefinedAt(2) // Dotty: true, Scalac: false

val c: PartialFunction[Int, String] = x => { println("foo"); x match { case 1 => x.toString } }
c.isDefinedAt(2) // Dotty: true
// Scalac:
//   error: type mismatch;
//     found   : Int => String
//     required: PartialFunction[Int,String]

I would argue that a function literal is a partial function that is always defined and both Dotty and Scalac are wrong.

Note: It works like this in Dotty because the compiler desugars partial function literals into function literals:

{ case 1 => x.toString } // becomes x => x match { case 1 => x.toString }

Then when the compiler generates the isDefined method, it doesn't differentiate between a tree that was originally a partial function literal and one that was a function literal

@Blaisorblade
Copy link
Contributor

  1. Dotty's clearly wrong (incompatible with Scalac) here, and in a way that could actually cause breakage for code that is pretty simple to write.

  2. The 2.12 spec (Sec. 6.23) mandates that x => x match { … } can be treated as a partial function. Spec changes would have to go eventually through the usual review. I'd say Dotty-only spec. changes on small details are mostly technical debt — we should just fix Dotty now.

  3. The situation for x => { x match { case 1 => x.toString } } is a bit fishier. Distinguishing between that and x => x match { case 1 => x.toString } seems clearly a bug — most phases should treat those trees as equal. The spec doesn't discuss this case, and I'll grant that's arguably a (minor) spec bug. But I think Scalac's precedent is enough to resolve doubts.

  4. I see your logic; but both x => x match { case 1 => x.toString } and x => { x match { case 1 => x.toString } } are very similar to { case 1 => x.toString }, so I see why the spec makes sense, even though here Scala is trying to be a bit smarter, but in a pretty fragile way.

Overall, I guess we should just follow the spec and Scalac here.

As usual, I'm biased to make things compatible unless we actually have a rewrite, and we should only bother with rewrites for cleanups that exceed some threshold of "importance".

@smarter
Copy link
Member

smarter commented Apr 9, 2018

After discussion during the weekly meeting, we agreed that we should just follow scalac here.

@odersky
Copy link
Contributor

odersky commented Jun 1, 2021

As per 2.13.5, scalac no longer flags

val c: PartialFunction[Int, String] = x => { println("foo"); x match { case 1 => x.toString } }

as an error. We now get

c.isDefinedAt(2) // Scala 2: true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants