Skip to content

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Aug 5, 2023

When "adapting" a function literal to a PartialFunction, don't ignore an explicit parameter type but use it directly.

Fixes scala/bug#4940

  ~ scala
Welcome to Scala 2.13.15 (OpenJDK 64-Bit Server VM, Java 23).
Type in expressions for evaluation. Or try :help.

scala> List(42).collect((x: String) => x match { case "42" => 27 })
                                                      ^
       error: type mismatch;
        found   : String("42")
        required: Int

scala>
:quit
  ~ skala
Welcome to Scala 2.13.16-20241122-141607-6782503 (OpenJDK 64-Bit Server VM, Java 23).
Type in expressions for evaluation. Or try :help.

scala> List(42).collect((x: String) => x match { case "42" => 27 })
                                         ^
       error: type mismatch;
        found   : scala.runtime.AbstractPartialFunction[String,27] with java.io.Serializable
        required: PartialFunction[Int,?]

scala>
:quit

For edge case syntax, List(42).collect((x: String) => x match { case "42" => 27 }), use the explicit param type to widen the type arg of the partial function. (Previously it was ignored.) Warn under -Xlint:infer-any.

@scala-jenkins scala-jenkins added this to the 2.13.13 milestone Aug 5, 2023
@som-snytt som-snytt force-pushed the issue/4940-numeric-anonfun branch from f1b44b9 to e770628 Compare August 7, 2023 09:51
@som-snytt som-snytt changed the title Typecheck functional literal before rewrite to PF Use type of functional literal param for PF synthesis Aug 7, 2023
@som-snytt som-snytt marked this pull request as ready for review August 7, 2023 22:13
@SethTisue SethTisue requested a review from lrytz August 8, 2023 23:29
@som-snytt
Copy link
Contributor Author

som-snytt commented Aug 9, 2023

I considered erroring outright, but I forgot to ask WWDD:

Welcome to Scala 3.3.0 (20.0.1, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> List(42).collect((x: String) => x match { case "42" => 27 })
-- [E007] Type Mismatch Error: -----------------------------------------------------------------------------------------
1 |List(42).collect((x: String) => x match { case "42" => 27 })
  |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |                 Found:    String => Int
  |                 Required: PartialFunction[Int, Int]
  |
  | longer explanation available when compiling with `-explain`
1 error found

scala> List(42).collect((x: Int) => x match { case "42" => 27 })
-- [E172] Type Error: --------------------------------------------------------------------------------------------------
1 |List(42).collect((x: Int) => x match { case "42" => 27 })
  |                                            ^^^^
  |                                            Values of types String and Int cannot be compared with == or !=
1 error found

scala> List(42).collect((x: Any) => x match { case "42" => 27 })
java.lang.AbstractMethodError: Receiver class rs$line$1$$anon$1 does not define or inherit an implementation of the resolved method 'abstract boolean isDefinedAt(java.lang.Object)' of interface scala.PartialFunction.
  at scala.PartialFunction.applyOrElse(PartialFunction.scala:214)
  at scala.PartialFunction.applyOrElse$(PartialFunction.scala:213)
  at scala.runtime.AbstractPartialFunction.applyOrElse(AbstractPartialFunction.scala:27)
  at scala.collection.immutable.List.collect(List.scala:267)
  ... 33 elided

I believe in sports such as chess, it is called a "draw".

Only the widened argument type is OK.

@som-snytt som-snytt marked this pull request as draft August 11, 2023 08:13
@som-snytt
Copy link
Contributor Author

I'll attempt to follow dotty and error hard before undrafting, but opinions welcome.

@lrytz
Copy link
Member

lrytz commented Aug 15, 2023

I'm in favor of a compiler error for List(42).collect((x: String) => ...), even if we can make it work. Or are there legit examples where it's useful, not as confusing?

@SethTisue SethTisue modified the milestones: 2.13.13, 2.13.14 Jan 18, 2024
@SethTisue SethTisue modified the milestones: 2.13.14, 2.13.15 Mar 13, 2024
@som-snytt som-snytt closed this Mar 20, 2024
@SethTisue SethTisue removed this from the 2.13.15 milestone Mar 21, 2024
@som-snytt
Copy link
Contributor Author

footnote

scala> List(42).collect((x: Any) => x match { case "42" => 27 })
1 warning found
-- [E030] Match case Unreachable Warning: ------------------------------------------------------------------------------
1 |List(42).collect((x: Any) => x match { case "42" => 27 })
  |                                            ^^^^
  |                                            Unreachable case
java.lang.AbstractMethodError: Receiver class rs$line$1$$anon$1 does not define or inherit an implementation of the resolved method 'abstract boolean isDefinedAt(java.lang.Object)' of interface scala.PartialFunction.
  at scala.PartialFunction.applyOrElse(PartialFunction.scala:214)
  at scala.PartialFunction.applyOrElse$(PartialFunction.scala:213)
  at scala.runtime.AbstractPartialFunction.applyOrElse(AbstractPartialFunction.scala:27)
  at scala.collection.immutable.List.collect(List.scala:267)
  ... 33 elided

@som-snytt
Copy link
Contributor Author

som-snytt commented Nov 20, 2024

just a rebase. We'll also address scala/bug#13000 (edit: in a separate PR).

The idea was to put the type args behind a bulwark of aliases to prevent warning, rather than tweak refchecks.

type In = argTp
type Out = resTp

@som-snytt som-snytt force-pushed the issue/4940-numeric-anonfun branch from 453e351 to 6782503 Compare November 22, 2024 14:16
@som-snytt
Copy link
Contributor Author

Roll back the "innovation" per the review comment and common sense.

Or are there legit examples where it's useful, not as confusing?

No, there is no such example.

@som-snytt som-snytt marked this pull request as ready for review November 22, 2024 15:41
@som-snytt som-snytt requested a review from lrytz November 22, 2024 15:41
@som-snytt som-snytt changed the title Use type of functional literal param for PF synthesis Use type of function literal param for PF synthesis Nov 22, 2024
@SethTisue SethTisue removed this from the 2.13.16 milestone Dec 12, 2024
@SethTisue SethTisue added this to the 2.13.17 milestone Dec 12, 2024
@som-snytt
Copy link
Contributor Author

github is munching on my push -f. Meanwhile, dotty now

-- [E172] Type Error: /home/amarki/snips/t4940.scala:3:67 ----------------------
3 |  val f: PartialFunction[String, Int] = (x: Int) => x match { case "x" => 3 }   // error
  |                                                                   ^^^
  |           Values of types String and Int cannot be compared with == or !=
-- [E172] Type Error: /home/amarki/snips/t4940.scala:5:65 ----------------------
5 |  val g: PartialFunction[String, Int] = (x: X) => x match { case "x" => 3 }     // error
  |                                                                 ^^^
  |             Values of types String and X cannot be compared with == or !=
-- [E007] Type Mismatch Error: /home/amarki/snips/t4940.scala:7:37 -------------
7 |  val m: PartialFunction[Int, Int] = (x: Double) => x match { case 3.14 => 3 }  // error
  |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |                                     Found:    Double => Int
  |                                     Required: PartialFunction[Int, Int]
  |
  | longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: /home/amarki/snips/t4940.scala:9:36 -------------
9 |  val g3: PartialFunction[Y, Int] = (x: X) => x match { case _: X => 3 }        // error
  |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |                                    Found:    X => Int
  |                                    Required: PartialFunction[Y, Int]
  |
  | longer explanation available when compiling with `-explain`

@som-snytt som-snytt force-pushed the issue/4940-numeric-anonfun branch from ef0b161 to 3075d16 Compare April 14, 2025 16:51
@som-snytt
Copy link
Contributor Author

som-snytt commented Apr 14, 2025

The flaky test.

# starting 49 tests in presentation
................................X................
!!  1 - presentation/t12308                       [output differs]
% diff /home/runner/work/scala/scala/test/files/presentation/t12308.check /home/runner/work/scala/scala/test/files/presentation/t12308-presentation.log
@@ -46,5 +46,6 @@ askType at Foo.scala(4,37)
 [response] askTypeAt (4,37)
 1
 ================================================================================
+Problem(RangePosition(t12308/src/Foo.scala, 43, 43, 48),A try without a catch or finally is equivalent to putting its body in a block; no exceptions are handled.,1,List())
 Problem(RangePosition(t12308/src/Foo.scala, 67, 67, 72),A try without a catch or finally is equivalent to putting its body in a block; no exceptions are handled.,1,List())
 Problem(RangePosition(t12308/src/Foo.scala, 109, 109, 114),A try without a catch or finally is equivalent to putting its body in a block; no exceptions are handled.,1,List())

##### Log file '/home/runner/work/scala/scala/test/files/presentation/t12308-presentation.log' from failed test #####

I wonder if that is also addressed by #10867

@som-snytt som-snytt changed the title Use type of function literal param for PF synthesis Use type of function literal param for PF synthesis [ci: last-only] Apr 14, 2025
@som-snytt som-snytt force-pushed the issue/4940-numeric-anonfun branch from 3075d16 to f187783 Compare April 14, 2025 20:40
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you!

@lrytz lrytz merged commit e5fcbbb into scala:2.13.x Apr 15, 2025
3 checks passed
@som-snytt som-snytt deleted the issue/4940-numeric-anonfun branch April 15, 2025 14:47
@som-snytt
Copy link
Contributor Author

@lrytz How many beers does this make that I owe you?

@lrytz
Copy link
Member

lrytz commented Apr 15, 2025

Someone thought they owed me a lot of beer one evening last week and it didn't turn out well for me 🍻

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

Successfully merging this pull request may close these issues.

Anonymous Function expression treated as a PartialFunction (spec needs broadened)
4 participants