Implement throw & catch statements

[juliusikkala]

The current status of the error handling mechanism is that throws in function signatures and try expressions work, throw only exists on the IR level (to allow try to re-throw) and catch is completely unimplemented.

This PR finishes throw such that it can be used directly, and aims to implement catch.

WIP: Might take a while. Finishing up throw seemed simple as it was mostly done already, but catch is a bit more complex. One non-trivial issue is the lifetime of variables that may be referenced from catch:

{
    let f = try func(); // throws SomeError
    let g = otherFunc();
    catch(e: SomeError) // btw, do we want to support "traditional" syntax here, with `catch(SomeError e)`?
    {
        // Should we be able to refer to `g` here? I don't think so.
        // So `catch` should probably either be unable to reference anything from its scope,
        // or only variables declared before the first `try` expression.
        // The former would probably be easier to handle.
    }
}

[csyonghe]

Thank for taking initiative to continue the error handling implementation! I think we can just made the current scope not visible to catch block for simplicity.

[juliusikkala]

I noticed one "gotcha" in the existing design of the Result<T,E> type. It appears that it requires the error type to be an integer, and the zero value is reserved for success. I didn't yet add proper checking for the integer-ness. This is also very easy to make subtle mistakes with:

enum MyError
{
    Failure1,
    Failure2
};

float f() throws MyError
{
    // Problem: the value of Failure1 is 0, so this throw gets
    // interpreted as a valid return value.
    throw MyError.Failure1;
}

export __extern_cpp int main()
{
    {
        let res = try f();
        printf("%f\n", res); // This is executed, with undefined value for 'res'
        catch(MyError err)
        { // This is not executed.
            printf("Caught an error %d\n", err);
        }
    }
    return 0;
}

Because I'd expect enums specifically being the most common way to create error types, maybe it'd be safer to reserve the maximum value of the integer type instead?

I'm also still missing a check for catch being the last statement in the block. I initially thought this wasn't necessary, but it's possible to declare variables after try and before catch that should be visible after catch if it isn't the last statement in the block.

[csyonghe]

We want to reserve 0 for success so that it works with COM interface convention.

The problem with using max instead of 0 for success is you will lose forward binary compatibility.

I think we should only allow enum types that inherit Error type to be used as error type, where Error is just a builtin enum that declares a general success code (0) and a general failure code (1).

[juliusikkala]

Inheriting from an enum doesn't seem to exist as a feature yet. Should I implement that? It doesn't seem terribly difficult, assuming it just means inheriting the underlying type and cases. I have a couple of other options in mind as well:

1.

interface IError
{
    bool isSuccess();
}

The above could allow non-integer error types as well, but admittedly doesn't enforce adherence to the COM interface convention.

2.
A new attribute [ErrorType] for enums, just like [UnscopedEnum] or [Flags]. Since there already are attributes that change how enums behave, this would fit right in line. It would make it easier to add a compiler error to prevent the user from accidentally overlapping an enum case with a success value. It would also allow the error types to not include an explicit success value, making it impossible to throw MyError.Success; and observe uninitialized return values.

[csyonghe]

Whenever possible, we should solve the problem with the type system instead of through new modifiers or attributes. We could add support for enum type inheritance or IError type. I think we will need something like IError in the long term so it is a good idea to consider implementing it now.

[csyonghe]

If we represent Result as Result<T, E:IError>, then the witnesses should be referenced in an IRResultType as the third operand, right?

[csyonghe]

I am fine with changing the syntax to try{}catch, but @tangent-vector may want to weight in here as she did the original design and I think there should be a legit reason that this is preferred.

[juliusikkala]

If we represent Result as Result<T, E:IError>, then the witnesses should be referenced in an IRResultType as the third operand, right?

I retried without KeepAliveDecoration and realized that I misremembered the details. The witness table itself is not removed from IR, but all of the functions inside it are removed before Result type lowering.

[tangent-vector]

I don’t fully remember my reasons for avoiding the traditional try/catch syntax, but a large part of it was probably just because the traditional syntax adds a pointless extra level of nesting in many cases, which makes code uglier than it needs to be. I’d be fine with supporting (or switching to) the traditional try/catch form, since that’s what our users are almost all going to be more familiar with it.

In a vacuum, I’d claim that a much better way to design a syntax for “catching” exceptions would be to write handlers at the point where they come into scope, and have them be in effect for the rest of the scope, just like any other binding. In toy language projects I’ve used on as a keyword for introducing handlers in general: both exception handlers and event handlers. In that model, you’d have:

let x = …;

on(e: SomeError) { … }

try x.doSomethingThatMighyThrow();

That approach avoids the introduction of a nested scope, marks the specific operations/lines that might throw, and makes it so that you can easily keep handlers close to the code that might branch to them.

(I’m aware that the historical reason for why catch clauses come at the end was because it keeps the code for the “happy path” uncluttered, but the collective experience programmers have had with exceptions makes me think that it’s best to not try to sweep the error cases under the rug so readily.

I support having an IError marker interface that user-defined errors must conform to, but I’m not actually a fan of making that interface enshrine the existence of a dedicated “not-an-error” value for error types.

The original idea was that Result<T,E> would semantically be a tagged-union sort of thing, and in the general case would use a bool to tell which case is present, but then the compiler might be able to employ different/optimized representations for specific cases of T and/or E.

For the initial version of error handling, we were primarily focused on making the COM HRESULT type work as an error in a way that would lead to interface method signatures working out nicely, but that should probably be seen as a very special-case feature rather than a template for how we’d want the error-handling features to work in the general case.

[juliusikkala]

I like Result<T, E> being a tagged union a lot. I'll make it do that instead (bool + anyValue that gets unpacked to either T or E).

If we get rid of the success values, then I don't really see the point of keeping IError around? It would make it easier to use custom error types and make the PR a lot simpler if I can remove it.

[juliusikkala]

I've now implemented the general case tagged union version of Result<T, E>, and it feels much cleaner IMO. I've also deleted IError for now, but I'm OK with reverting that if there still is some need for it.

I'm a bit out of the loop on the HRESULT stuff. I assume this is about slang-ir-marshal-native-call.cpp and [DllExport]? It currently uses 0 as a return value when there is no error. This won't work in the general case, when the error type can be anything. A couple of ideas to handle this corner case:

• Return value could just either be S_OK or E_FAIL depending on the boolean tag, with the actual error returned with a pointer parameter just like the original return value already is.
• Create a dedicated builtin HRESULT type that is the only allowed error type with a [DllExport] function.
• Or both, where the HRESULT is a special case and converting the boolean tag to S_OK or E_FAIL would be a fallback.

Regarding the syntax, I feel that adding an extra level of nesting is often necessary anyway, because the catch must occur last in scope:

int val;
{
    val = try getDifficultValue();
    catch(err: Error)
    {
        val = getFallbackValue();
    }
}
// continue doing something with val

The classical style would actually have less indented code in total in these cases, as the catch doesn't need to be nested in the block:

int val;
do // using `do` here to match Swift and avoid overlap with the `try` keyword below.
{
    val = try getDifficultValue();
}
catch(err: Error)
{
    val = getFallbackValue();
}
// continue doing something with val

Still, you're completely right that there are also many cases where this isn't needed, and the current syntax does have less nesting in those cases. I don't have any strong feelings towards either option, and supporting both also seems viable, but it might seem "messy" if there's two ways to do a very similar thing.

[csyonghe]

If catch has to appear as the last statement in a scope, then we should definitely be doing do{}catch{} syntax. However I wonder why that is the case. We can make catch appear anywhere, and it applies to everything from the beginning of the scope to the start of the catch.

[csyonghe]

Looks good. But we want to have an official opinon on whether error type should include a value for success.

[csyonghe]

Reading the implmementation, I am leaning towards that we just support do{}catch(){} syntax for simplicity of the lowering logic.

Do we want to also support a catch-all clause, e.g. catch{}?

I think if we call parseModernParamDecl, it should handle both the modern and legacy syntax cases for a parameter.

[juliusikkala]

If catch has to appear as the last statement in a scope, then we should definitely be doing do{}catch{} syntax. However I wonder why that is the case. We can make catch appear anywhere, and it applies to everything from the beginning of the scope to the start of the catch.

I initially thought so as well, and thought that this would be super convenient. However, I reckon the limitation is given in the proposal for a very good reason. It's hard to define how catch should work in many cases, e.g.:

int a = try something();
int b = try other();
catch(err: MyError)
{
    ...
}
int c = a + b;

Now, the block introducing b does not dominate c. If something() throws, it'll jump over the b line and go straight to c, which then tries to use b. With do{}catch{} and last-in-block catch, we can avoid these circumstances.

I'm also in favor of a catch-all clause, I think it would be simple to implement and could often be practical.

[csyonghe]

Can you also create a PR to the spec repo to update the error handling proposal so it reflects what’s being implemented here?

[csyonghe]

Looks good to me! This is a significant enhancement to the language. Thank you for finalizing and implementing the error handling feature for Slang!