Flow-typing does not record non-null info for try-catch blocks

[HarrisL2]

Compiler version

3.7

Minimized code

//> using options -Yexplicit-nulls

def main() =
  var cache: String | Null = null
  try {
    cache = ""
  } catch {
    case e =>
      throw e
  }
  cache.trim()

Output

-- [E008] Not Found Error: local/flow.scala:10:8 -------------------------------
10 |  cache.trim()
   |  ^^^^^^^^^^
   |  value trim is not a member of String | Null.
   |  Since explicit-nulls is enabled, the selection is rejected because
   |  String | Null could be null at runtime.
   |  If you want to select trim without checking for a null value,
   |  insert a .nn before .trim or import scala.language.unsafeNulls.

Expectation

If the try block sets a variable to non-null, and the catch block does not retract it, we can expect the variable outside of the try-catch to be non-null as well

[noti0na1]

This is not safe, for example:

try 
  f() // throw inside f
  cache = "" 
case
  ...

Since we don't know at which point the the exception is thrown in the body, we don't trust any notnull info, and we have to collect any reference that is once retracted.

[noti0na1]

See examples in #21619

[olhotak]

But in this case, the catch block cannot complete normally, only exceptionally, since the only case is a throw. This means that if execution reaches the code after the try - catch, no exception could have been thrown in the try clause. Therefore the code after the try - catch can assume that the try completed normally.

[sjrd]

Edit: never mind; I misunderstood the comment.

A try that always completes normally is useless. You can remove the catch entirely. IMO it's not worth adding rules to flow typing to make this use case better, because it's not a use case.

A try that always completes abnormally is not totally useless. That's not what we have here though.

[noti0na1]

In order to use the NotNull info of the body after try, we have to ensure that all catch cases terminate "exceptionally" (by return, throw, etc.)

[sjrd]

Ah right. If the type of all the handlers is Nothing, then you could use the NotNull info of the body after the try.

[olhotak]

It doesn't have to match the full space of the possible exception. It's enough to ensure that they all terminate exceptionally. If the exception is not matched, it will not be caught and continue exceptionally, but will not reach the code after the try-catch.

The point is, the flow state after the try-catch should be: (normal flow of end of try) join (normal flow of end of case 1) join (normal flow of end of case 2) join ...

The flow state at the beginning of each case should be: non-asserted flow of try.

The current implementation imprecisely reuses the non-asserted flow of try for the flow state after the try-catch. That is sound but imprecise.

If that was all there was to it, a simple fix to this issue would be to correct the flow state after the try-catch to the join above. However, there could also be a finally clause, which adds more complexity.