Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add compile error for ignoring error #772

Closed
andrewrk opened this Issue Feb 11, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@andrewrk
Copy link
Member

commented Feb 11, 2018

test "aoeoue" {
    _ = foo();
}
fn foo() !void {
    return error.OutOfMemory;
}

This passes but it should be a compile error. To ignore errors, use foo() catch {}.

@thejoshwolfe

This comment has been minimized.

Copy link
Member

commented Mar 18, 2019

test "asdsfd" {
    const whatever = foo();
    // several lines of code... and then:
    bar(whatever);
}
fn foo() u32 {
    return 0;
}
fn bar(x: u32) {
    _ = x;
}

Then if you replace the definition of foo with:

fn foo() !u32 {
    return error.OutOfMemory;
}

You get an error in the call to bar(), but I believe the error should be on the assignment to whatever, since that's where you should introduce the try to resolve the error. There's no telling how far away the error will be with a situation like this; it might be in a different function or in a different file.

I propose that when a function returns an error union, you must either use try or catch to explicitly deal with the error, or you must use some explicit syntax to preserve the error union type.

test "asdsfd" {
    const whatever = foo(); // ERROR: error union not allowed in type inference
    const explicit: !u32 = foo(); // ok
    const explicit2 = (!u32)(foo()); // also ok?
}
fn foo() !u32 {
    return error.OutOfMemory;
}

This would be a case of the inferred error set !u32 appearing outside a function return type. I don't believe this should inflict any significant implementation complexity or cognitive burden, since this is already possible in status quo with regular type inference. This means that the following would not be allowed:

test "asdsfd" {
    var explicit: !u32 = undefined; // ERROR: can't infer error set
    explicit = foo(); // too late

    var explicit2 = (!u32)(undefined); // ERROR: can't infer error set
    var not_an_error = (!u32)(0); // ERROR: inferred error set must have at least one error

This might be a bad idea in some cases, such as when generic code gets error union types in place of a T:

// in ArrayList
    fn get(self: Self, index: usize) T {
        self.checkIndex(index);
        const value = self.uncheckedGet(index); // This line
        return value;
    }

The This line line could get an error if T was !u32. With my proposal thusfar, the workaround would be:

        //const value = self.uncheckedGet(index);
        const value: T = self.uncheckedGet(index);

That's pretty bad. I don't have an elegant solution to that problem at this time. 🤔

@thejoshwolfe

This comment has been minimized.

Copy link
Member

commented Mar 18, 2019

I would also like to say that I have suffered from the problem where the distance between foo() missing a try and the error ending up on the call to bar() many times. This is a very common pattern in my experience. I believe every single zig project I've worked on has run into this problem at least 3 times.

@andrewrk andrewrk added the breaking label Mar 23, 2019

@andrewrk andrewrk closed this in 64dddd7 Mar 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.