-
Notifications
You must be signed in to change notification settings - Fork 21
Anonymous Function expression treated as a PartialFunction (spec needs broadened) #4940
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
Comments
Imported From: https://issues.scala-lang.org/browse/SI-4940?orig=1 |
Kipton Barros (kbarros) said: |
@lrytz said: |
@som-snytt said: |
@SethTisue said: |
@som-snytt said: SKALA 2.11.7-20150616-093756-43a56fb5a1> val g: PartialFunction[String, Int] = (x: Int) => x match { case "x" => 3 }
g: PartialFunction[String,Int] = <function1> |
someone want to take a look and see whether this should remain open? I wonder what Dotty does here. |
This one compiles now 😄
In dotty it doesn't, and Som's variant doesn't work in Dotty:
I don't think this issue is worth fixing (but I'm fine with spec changes, less sure about compiler changes). voteForClose += 1 |
I think the bug where Scala 2 ignores the type of the parameter is just a bug. It overdoes the equivalence of A different view:
I think my sense that 8.5 already justifies the working behavior is:
Obviously, |
relevant passage from SLS 6.23.1 that I don't recall having seen before:
|
Another weird case: scala> val f: PartialFunction[String, Int] = _.toInt match {
case 0 => 0
}
f: PartialFunction[String,Int] = <function1> This is the resulting private[this] val f: PartialFunction[String,Int] = ({
@SerialVersionUID(value = 0) final <synthetic> class $anonfun extends scala.runtime.AbstractPartialFunction[String,Int] with java.io.Serializable {
def <init>(): <$anon: String => Int> = {
$anonfun.super.<init>();
()
};
final override def applyOrElse[A1 <: String, B1 >: Int](x$1: A1, default: A1 => B1): B1 = (scala.Predef.augmentString(x$1).toInt: Int @unchecked) match {
case 0 => 0
case (defaultCase$ @ _) => default.apply(x$1)
};
final def isDefinedAt(x$1: String): Boolean = (scala.Predef.augmentString(x$1).toInt: Int @unchecked) match {
case 0 => true
case (defaultCase$ @ _) => false
}
};
new $anonfun()
}: PartialFunction[String,Int]);
<stable> <accessor> def f: PartialFunction[String,Int] = $iw.this.f |
So the original ticket is about the compiler interpreting the 6.23.1 language too loosely, but it appears to me that @nigredo-tori has found that the compiler is WAY looser than we realized. Rather to my astonishment, Dotty (0.27.0-RC1) accepts it as well: Starting dotty REPL...
scala> val f: PartialFunction[String, Int] = _.toInt match { case 0 => 0 }
val f: PartialFunction[String, Int] = <function1>
scala> f.isDefinedAt("1")
val res0: Boolean = false |
Seth was the quoting the spec added by Dale. That's why Dale was grinning in his previous comment. Possibly, Dale intended In the context of the preceding discussion, I think
My pre-pandemic comment, of which I have no recollection, would suggest
|
I've not tested this or anything, but I'd put some easy money on that possibly being because of https://github.com/scala/scala/blob/df25f7ab79baa3d566755bf41572cb8f6454a9c3/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala#L611-L614
i.e. #7868, i.e. it's interacting with the implicit numeric widening in the typer? |
@dwijnand, val f: PartialFunction[String, Unit] = _.toUpperCase match {
case "" => ()
} UPD: although I can see that the code you've quoted is not specific to widening, so please disregard this comment. |
Regarding @som-snytt's 5 year old example:
That looks like a clear bug. I don't really see how most other examples in this issue are problematic. The ones where the input is transformed before the pattern match are pretty liberal but whether that's a real problem is debatable. |
It's actually kind of a feature IMHO:
|
How does that desugar exactly? |
$ scala -Vprint:patmat -e 'List("1", "two", "3").collect { x => scala.util.Try(x.toInt) match { case scala.util.Success(int) => int } }'
[[syntax trees at end of patmat]] // scalacmd7634219973344927097.scala
object Main extends scala.AnyRef {
def main(args: Array[String]): Unit = {
final class $anon extends scala.AnyRef {
scala.collection.immutable.List.apply[String]("1", "two", "3").collect[Int](({
@SerialVersionUID(value = 0) final <synthetic> class $anonfun extends scala.runtime.AbstractPartialFunction[String,Int] with java.io.Serializable {
final override def applyOrElse[A1 <: String, B1 >: Int](x: A1, default: A1 => B1): B1 = {
case <synthetic> val x1: scala.util.Try[Int] = (scala.util.Try.apply[Int](scala.Predef.augmentString(x).toInt): scala.util.Try[Int] @unchecked);
case5() {
if (x1.isInstanceOf[scala.util.Success[Int]]) {
<synthetic> val x2: scala.util.Success[Int] = x1.asInstanceOf[scala.util.Success[Int]];
val int: Int = x2.value;
matchEnd4(int)
} else
case6()
};
case6() { matchEnd4(default.apply(x)) };
matchEnd4(x: B1) { x }
};
final def isDefinedAt(x: String): Boolean = {
case <synthetic> val x1: scala.util.Try[Int] = (scala.util.Try.apply[Int](scala.Predef.augmentString(x).toInt): scala.util.Try[Int] @unchecked);
case5() {
if (x1.isInstanceOf[scala.util.Success[Int]])
matchEnd4(true)
else
case6()
};
case6() { matchEnd4(false) };
matchEnd4(x: Boolean) { x }
}
};
new $anonfun()
}: PartialFunction[String,Int]))
};
new $anon();
()
}
} |
It seems that we have a rough consensus, here and on scala/scala#10483 , that the behavior is plausibly desirable, and since Scala 3 allows it too, we should leave well enough alone in Scala 2 as well. If someone were to succeed in persuading the Scala 3 folks to deprecate or disallow, we could then warn in Scala 2 under |
On second thought, reopening but reclassifying as a spec ticket. |
The linked PR shows the bug |
Ah, I hadn't seen scala/scala#10486 yet. I'm still comfortable with the disposition of this ticket though — 10486 is narrower, though under the same umbrella. |
I think we should align with Scala 3, it does support operations on the scrutinee but gives a clear error message for |
The following compiles, although I think the conversion to type PartialFunction from this anonymous function expression is disallowed (there is no mention in SLS, Section 6.23, Anonymous Functions)
Section 8.5, Pattern Matching Anonymous Functions, introduces the syntax
{case ...}
for defining a PartialFunction. If a Function type is expected, then the expression{case ...}
is reinterpreted asx => x match {case ...}
. The strange thing here is that the opposite conversion is happening: a Function expressionx => x match {case ...}
is given, and the compiler is interpreting it as a PartialFunction,{case ...}
.Here's another example:
If the Function expression gets a little more complicated, the conversion from Function to PartialFunction fails:
Apologies if I am misunderstanding the Spec.
The text was updated successfully, but these errors were encountered: