Proper try keyword

[smores56]

A working version of try implemented as a keyword instead of a desugaring. This allows users to get better type errors when they use try incorrectly.

Though we plan to likely drop try once we move to static dispatch, this implementation should be easy to migrate to a ? version without too much effort, meaning this is "future-proof".

• No new parsing, just reuse of the old try parsing, minor update to desugaring
• Consolidated constraining of function returns/early returns/try into a helper function
• Added a new TryTargetConstraint that validates that something is Result-shaped during the try target validation process
• Various reporting fixes

[smores56]

We used to be printing

3 errors and 0 warnings found in <ignored for test> ms
.

now we get

3 errors and 0 warnings found in <ignored for test> ms.

[smores56]

I tried keeping println! and putting the period at the end of this statement, but there are multiple places where we append text after this line that would break if we always put the period here. For another PR 🤷🏼

[smores56]

I was surprised to see this opaque definition included, but it's here because we now include the full def body in these diagnostics. This test's body actually gets turned into:

main =
    A := U8
    List.map [1u16, 2u16, 3u16] @A

So the inclusion of the opaque type makes sense, IMO.

[lukewilliamboswell]

Looks good to me 😄

[smores56]

These are duplicated versions of try-based tests, meaning we now have separate tests for try and return

[smores56]

This validates that we see the full pipe instead of just the readFile filePath arg, which was confusing.

[smores56]

These were previously swapped, meaning we wouldn't show an expression and some context, but just the expression. Errors should be more useful (a.k.a. have more context) now.

[smores56]

@ayazhafiz I copied the try reporting tests and made return tests to satisfy your second set of comments from the #7238 PR on pure statements with returns in them.

On the first set of comments: for me to handle improving error reporting on problematic style of statement, e.g.

foo = \{} ->
  if True then
    return 1
  else
    2

  3

You suggested the following, which I'm not understanding:

I wonder if a better approach would be to infer the type of the statement like

if True then return 1 else return 2

as the unbound type and then just run the ExpectEffectful constraint on that. Unifying the unbound type to [Ok {}, Result a] where a is also unbound will satisfy trivially. If it doesn't unify, then you know that either it's not effectful or there is a path where the expression returns.

A couple of questions:

• This one-line example shouldn't matter because a "switch" statement like if or when that has return for all branches will unify to the unbound type, so we don't get any errors with it. Does your proposed solution still work if one of the branches erroneously returns a number instead of {}? If so, can you explain a bit why it would work?
• I don't see a way to use an existing constraint to output a different reporting error. So if I'm trying to use the ExpectEffectful constraint as above and it fails, it fails in solve/src/solve.rs and I can't control the error from constrain/src/expr.rs. Any ideas on managing which report to show when running the same constraint?
  • If not, no worries, I'm sure it's been a while since you touched this part of the code

[ayazhafiz]

@smores56 sorry the opaque initial comment. What I am suggesting is distinguishing between return x as a value, and the type of x that applies to the return constraint. Specifically, return x as a value should be the unbound type ("flex var"), while return x adds a constraint that typeof (return type of function) ~ typeof x.

With the example above, I would suggest adding these constraints

# All variables that are initially unbound (flex vars) are prefixed with a below
# Suppose we define a1 return type of `foo`
foo = \{} ->
  # Define `a2` to be the type of this `if/else` expression
  if True then
    # Add constraint: a1 ~ typeof 1
    # Define typeof (return 1) = a3, a new unbound var
    # Add constraint a2 ~ a3
    return 1
  else
    # Add constraint a2 ~ typeof 2
    2

  # Add constraint a1 ~ typeof 3
  3

After solving, the types will be

foo : {} -> Num a (a is generalized)
foo = \{} ->
  # type of this expression is Num b, because a2 ~ typeof 2 ~ a3 => a2 = a3 = Num b
  if True then
    return 1
  else
    2

  3

Now we can see that we should emit a warning because the standalone if/else produces a value (of type Num b) but is not bound to a variable.

I suppose that for your case you actually want to unify the standalone expression to the empty record to see whether the warning should be emitted, right? Maybe my note about the constraints was wrong. I'm not sure what constraint is most useful, I just mean to say taking a type-directed approach to determine whether the statement could evaluate to a value that needs to be bound is probably better than walking the AST (and also handles cases where the statement calls a function that never returns, for example the infinite loop)

If we take a couple other cases this works just as well

foo = \{} ->
  # type of this expression is Str, because typeof (return 1) ~ typeof "" = a1 ~ Str => a1 = Str, a1 is unbound
  # Emit a warning because Str !~ {}
  if True then
    return 1
  else
    ""

  3

foo = \{} ->
  # type of this expression is unbound a1, because typeof (return 1) ~ typeof (return 2) = a1 ~ a2 => a1 = a2, a1 a2 are unbound
  # No warning because a1 ~ {} => a1 = {}
  if True then
    return 1
  else
    return 2

  3

divergent : {} -> *
divergent = \{} -> divergent {}

foo = \{} ->
  # type of this expression is unbound a1, because typeof (divergent {}) ~ typeof (return 2) = a1 ~ a2 => a1 = a2, a1 a2 are unbound
  # No warning because a1 ~ {} => a1 = {}
  if True then
    divergent {}
  else
    return 2

  3

[lukewilliamboswell]

This is a really nice improvement for try, I particularly enjoyed seeing the new error reporting. 😄

[smores56]

Thanks @lukewilliamboswell ! Another PR, coming at you in a few minutes.

[lukewilliamboswell]

Man... you're too efficient