catch case fails to warn in case of unchecked bounded type parameter

[scf37]

Compiler version

3.3.1

Minimized code

def handle[E <: Exception](f: => Unit): Option[E] = 
  try 
    f
    None
  catch case e: E => Some(e)

val r: RuntimeException = handle[RuntimeException](throw new Exception()).get

Output

java.lang.ClassCastException: class java.lang.Exception cannot be cast to class java.lang.RuntimeException (java.lang.Exception and java.lang.RuntimeException are in module java.base of loader 'bootstrap')

Expectation

Fail compilation because type of E is not known in compile time, allow compilation for inline method (or maybe also by ClassTag in scope?).

[szymon-rd]

The catch case e: E => Some(e) should not compile. It's even weirder that it tries to cast to RuntimeException, given that erasure should happen and there is no TypeTest instance.

[scf37]

inline def handle currently catches correct exception type

[dwijnand]

This should either compile and emit an unchecked warning, or it shouldn't compile. Perhaps compare how Typer and Erasure treats x match { case e: E => .. } and mirror that.

[scala-center-bot]

This issue was picked for the Issue Spree No. 40 of December 5th, 2023 which takes place in 6 days. @smarter, @SethTisue, @hamzaremmal, @KuceraMartin will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here.

[SethTisue]

Scala 2.13: "warning: abstract type pattern E is unchecked since it is eliminated by erasure". This is the correct behavior, IMO: compile, but warn.

At runtime you get the same java.lang.ClassCastException, of course.

[SethTisue]

The group agrees that the right course of action is to issue a warning, like Scala 2, the same warning that's issued for this comparable code involving match instead of try:

scala> def handle[E <: Exception](e: Exception): E = e match { case e2: E => e2 }
1 warning found
-- Unchecked Warning: ----------------------------------------------------------
1 |def handle[E <: Exception](e: Exception): E = e match { case e2: E => e2 }
  |                                                             ^
  |the type test for E cannot be checked at runtime because it refers to an abstract type member or type parameter

As for difficulty... it's not that easy 😭 But it should still be spree-sized.

Background: sometimes try/catch in user code is rewritten to a pattern match. But not always. TryCatchPatterns "compiles the cases that can't be handled by primitive catch cases as a common pattern match", where "primitive" catch cases mean those that can be handled by the JVM's catch instruction. If the "common pattern match" route is taken, with a TypeApply node in the tree, then we get the unchecked warning. The bug comes in the primitive-catch-case scenario. In that code path, TypeTestCasts#whyUncheckable is never called so the warning isn't issued.

So the solution plan we (mostly Guillaume 😅) have in mind is to add a case to Erasure that looks for Bind nodes, since that's how the primitive catch case is represented, as CaseDef(Bind(...)). When it sees one, it should call into TypeTestCasts, except we'll need to refactor TypeTestCasts a bit because its current entry point, interceptTypeApply, is specific to TypeApply nodes.

In the test case(s), we should make sure that @unchecked can still be used to avoid the warning.