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

Should fallible stdlib methods return Option? #159

Closed
phated opened this issue Jun 27, 2020 · 10 comments
Closed

Should fallible stdlib methods return Option? #159

phated opened this issue Jun 27, 2020 · 10 comments

Comments

@phated
Copy link
Member

phated commented Jun 27, 2020

I was looking at some methods in lists.gr and noticed that hd/tl/etc have a fail call in them. Once #158 lands, do you think that these methods should be changed to return Options? I really appreciate that from stdlibs I've used in the past.

@ospencer
Copy link
Member

I'm a big fan of having two versions, like hd and hdOpt, personally.

I would love for Grain to adopt a pattern where a function name that ends in ? would return an option, so we could have hd and hd?. Just a thought.

But otherwise, I think for these we should have a version that throws and a version that returns an option.

@phated
Copy link
Member Author

phated commented Jun 28, 2020

I'm not a fan of having an exception and option returning version of the same function. It definitely feels like "legacy" problems in the OCaml stdlib.

As for ?, I'd be careful with that symbol as part of the language, I think a lot of people coming from something like Rust would expect that to be an unwrap-or-early-return.

@ospencer ospencer changed the title Question: Should fallible stdlib methods return Option? Should fallible stdlib methods return Option? Sep 17, 2020
@ospencer
Copy link
Member

ospencer commented Oct 3, 2020

This has been open for a while... I think I vote that they just return options. Thoughts @grain-lang/core?

@ohana54
Copy link
Contributor

ohana54 commented Oct 3, 2020

I'm not grain-lang/core, but I vote for options as well :)
From my experience with reasonml, having to be careful with which functions can throw was not ideal...

@ng-marcus
Copy link
Contributor

My preference is for options too. Forcing the programmer to consider all potential code paths is a good thing and has raised the quality of my team's code a huge amount. It can add a level of unwrapping overhead, but the alternative is unhandled exceptions in production.

Having both forms I'm more ambivalent about - it frequently leads to code review arguments that the value being used has already been checked (e.g. array length and unsafe array access), but that can nearly always be recoded to remove the manual check and use the safe check version anyway.

@chriskrycho
Copy link

Strongly agree with returning Option. If you know that there’s nothing to check—for example because you just checked it in some way—you can always do the equivalent of this from Rust:

if (some_vec.len() > 1) {
    let item = some_vec
        .pop()
        .expect("we just checked that there's at least one element");

    // ...
}

This ends up working very well in practice: it means the normal path is the one that just checks the behavior, while giving end users an alternative which will blow up if they’re wrong… but they have to explicitly opt into it. There are times when this is the right thing to do, but because just using pattern matching or functional composition (map, andThen, etc.) is easy, you end up usually just doing it that way:

some_vec
    .pop()
    .and_then(|item| {
        // ...
    });

Rust (and Swift) also gives syntax sugar for working with optional types which is extra nice here:

if let Some(item) = some_vec.pop() {
    // ...
}

This is a particularly good fit for a language like Rust or Swift which still supports (even encourages) an imperative style of coding for performance reasons, but it also means that in practice it's extremely rare that I feel the need to .unwrap() or .expect() when working with Option or Result types in Rust code.

@chriskrycho
Copy link

Also, seconding @phated’s notes about ?. I think where Rust has landed on it is extremely nice, though I’d also strongly encourage folks to take a long look at the discussions around error-handling in Rust over the last few years before committing to any particular move in that space. Perhaps one of the most interesting posts there is this one, which describes a system for a generalized approach to effects—in Rust’s context, things-which-implement-the-Try-trait (mostly meaning Result and Option, where the ? sugar is used), Futures and async/.await, and unsafe.

None of these would be directly applicable to Grain, as they emerge in the context of Rust’s fundamentally different design constraints—especially the need for zero-cost primitives here because of the key target of being a non-GC, high-performance language and its constraints around backwards compatibility as a stable language! But the Rust community has had a bunch of very good discussions about the tradeoffs between certain approaches to error-handling, so worth pulling on those threads a bit.

I’ll also note that the Swift approach to both Optional and Result and errors is a fascinating dual to Rust’s, because so very different.

First, it layers in considerable sugar for Optional—so much so that in my experience a lot of day-to-day Swift developers don’t actually grok that optionals are an Optional<T> enum type (same as Rust’s Option<T>, and only really think of it in terms of the annoyance of having to deal with ?s everywhere: a sense of the compiler yelling at them, never advancing to the sense of sum-types-as-tool.

As regards error-handling, Swift has both exceptions and (as of Swift 5) Result and uses them in different ways and different places. I… hate it, to be honest. 😂 There are reasons they chose exceptions, including the fact that exceptions can improve performance for the happy path, and they at least made the fact that functions are fallible explicit, but they’re one of the most dynamically-typed parts of the language, and I think it also ends up preventing teams from gaining the benefits in reasoning about all the scenarios in the code that having a Result type as the first-class choice for error-handling provokes.

Then there are very rigorously-no-exceptions approaches like Elm’s, where everything is Maybe, Result, Task, etc. I personally love that, but the lack of escape hatches can definitely be painful for people at times.

My overall preference is for exceptions to exist but to be effectively-uncatchable. This is (kind of) how panicking works in Rust: if you panic!("This should be impossible!"), it kills the thread, full stop. (Technically you can catch panics, but only in certain ways and conditions, and it's actually possible to compile code so that you can't catch them and a panic turns into an abort.) This basic approach means that you have the escape hatch of go ahead and just kill everything and dump a stack trace when that's actually the right thing—unrecoverable errors—but for all expected errors, you use Result.

(There’s yet more to say on the design questions in that space as well: especially around things like how to layer Result types through your system, as in sufficiently-large systems it’s common to end up with one massive list of every error which can occur, and that’s not awesome either. But this comment is already long enough. 😆)

@ospencer
Copy link
Member

ospencer commented Oct 5, 2020

Everyone has great points here! I remember us briefly talking about having if let in Grain, and I feel like it'd be really fitting.

Chris also made me remember to say this: since it'll be a little while before we've got first-class support for exceptions in WebAssembly, it makes even more sense to stay away from exceptions.

@phated
Copy link
Member Author

phated commented Oct 14, 2020

Yeah, so I'm pretty much onboard with the things discussed here. If the stdlib uses Option and Result everywhere, it should become common place for end-users as well (often coding patterns are inferred from the stdlib).

I think that we need some better utility in the language before we do that, such as ? for unwrap-or-throw and if let destructuring.

@phated
Copy link
Member Author

phated commented Dec 12, 2020

I believe this is done now. There are some List methods that still fail on negative number inputs and stuff, but since that is argument validation instead of result validation, I left it in.

I also opened #464 to document these patterns.

@phated phated closed this as completed Dec 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants