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 exception handling to Ruru #49

Open
d-unsed opened this issue Jan 3, 2017 · 3 comments
Open

Add exception handling to Ruru #49

d-unsed opened this issue Jan 3, 2017 · 3 comments

Comments

@d-unsed
Copy link
Owner

d-unsed commented Jan 3, 2017

handle_exceptions! {
    begin {
        // ...
    } rescue TypeError => e {
        // ...
    } rescue ArgumentError => e {
        // ...
    } rescue => e { // StandardError
        // ...
    } ensure {
        // ...
    }
}

Also there should be a way to call it without ensure (as well as ensure without rescue blocks). Possibly, separate macros are required.

@eternaleye
Copy link

eternaleye commented Jan 11, 2017

Hm, not entirely sure I'm fond of the syntax - it's very Ruby, and very not Rust. In particular:

  • The repetitive rescue keywords seem rather superfluous
  • The repetitive e is similarly so
  • The overall behavior is close to match, but the syntax doesn't 😛

As a strawman:

handle_exceptions! {
    begin {
        // ...
    } rescue e {
        TypeError => /* ... */,
        ArgumentError => /* ... */,
        _ => /* StandardError */,
    } ensure {
        // ...
    }
}

This would significantly reduce the boilerplate, and much more closely mimic the syntax of match. There are some subtleties - e is an ident being bound rather than an expression being matched, and TypeError et al aren't quite patterns, but the behavior is (overall) quite easy to unpack.

An alternate strawman is even more match-like:

handle_exceptions! {
    begin {
        // ...
    } rescue {
        TypeError(e) => /* ... */,
        ArgumentError(_) => /* ... */,
        e => /* StandardError */,
    } ensure {
        // ...
    }
}

EDIT: Also, you can always just have three matchers in the same macro - one with rescue and ensure, one with only rescue, and one with only ensure. That's one advantage of having delimiting keywords like that. Stuff like this makes me wish there was a ? quantifier to go with * and +, but oh well.

@KiChjang
Copy link

Does this issue also include unifying the exception types with the existing types? It is totally valid Ruby code to raise an exception as a "return value", e.g.

def foo(x)
  raise ArgumentException unless x.is_a? Numeric
  x + 5
end

Perhaps in Rust land, every Ruby method should return a Result<T, Exception> instead of simply T?

@danielpclark
Copy link
Contributor

danielpclark commented Feb 18, 2018

Perhaps in Rust land, every Ruby method should return a Result<T, Exception> instead of simply T?

I'm in favor of this. I think Exception can be a simple alias to AnyObject and work well. I've already written this format for the eval method #79 :

pub fn eval(string: &str) -> Result<AnyObject, AnyException> {
    vm::eval_string_protect(string).map(|v|
        AnyObject::from(v)
    ).map_err(|_|
        AnyException::from(vm::errinfo())
    )
}

Now with PR #88 I've implemented this for send and public_send called protect_send and protect_public_send.

I've created an RFC for this subject #91 and I've submitted a PR to add AnyException #93 which doesn't currently change anything in ruru, it's just a bonus.

Note: For the AnyException object it's best to use rb_exc_raise and not rb_raise.

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

4 participants