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

Expand `try!` macro with additional case #1393

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
8 participants
@hauleth
Copy link

hauleth commented Dec 5, 2015

As it is suggested in Reddit post I suggests try!($expr => return)
macro to return earlier from function returning () and discard Err(…) value.

As honestly I am not sure about this feature, but I think that community should
decide about it.

Rendered

@alexcrichton alexcrichton added the T-libs label Dec 5, 2015

@withoutboats

This comment has been minimized.

Copy link
Contributor

withoutboats commented Dec 5, 2015

I personally would be more interested in seeing this solved using trait based exception handling, and deprecating try!.

@hauleth

This comment has been minimized.

Copy link
Author

hauleth commented Dec 7, 2015

@withoutboats IMHO this is bad idea. Error handling in Rust is IMHO great and we should not change that. Language itself should know nothing about error handling, it should be left as is with some additional helpers like this one.

@vadimcn

This comment has been minimized.

Copy link
Contributor

vadimcn commented Dec 8, 2015

Can you please expand the "Motivation" section to actually include motivation? (The Reddit post doesn't really explain it either, but even if it did, the RFCs are supposed to be self-contained). Is it to enable use of try! in functions that don't return a Result (e.g.main)?

@hauleth

This comment has been minimized.

Copy link
Author

hauleth commented Dec 8, 2015

@vadimcn exactly. But as I have written in RFC, I am not convinced that this should land in libstd as silently dropping errors is bad idea.

@vadimcn

This comment has been minimized.

Copy link
Contributor

vadimcn commented Dec 8, 2015

@hauleth: Please put this info into the RFC. It should be clear what and why is being proposed without reading through the comments thread.

As for the idea itself, I think it has some merit. It isn't really a silent drop, since return shows up in the source. It would allow us to use try! in examples without having to put up disclaimers that this code cannot put in main directly.

I am not sure I like the proposed syntax though: => does not exactly convey the idea of "this happens on error". IMHO, something like try!(something() else return) would read better.

@hauleth

This comment has been minimized.

Copy link
Author

hauleth commented Dec 8, 2015

Unfortunately try!($expr else return) is illegal as expr can be followed only by =>, , or ;.

@defuz

This comment has been minimized.

Copy link

defuz commented Dec 8, 2015

I was waiting when someone will offer it, but I'm wondering why you have not offered more general variant, that allows you to do any action in case of an error:

macro_rules! try {
    ($expr:expr) => (match $expr {
        $crate::result::Result::Ok(val) => val,
        $crate::result::Result::Err(err) => {
            return $crate::result::Result::Err($crate::convert::From::from(err))
        }
    });

    ($expr:expr => $err:expr) => (match $expr {
        $crate::result::Result::Ok(val) => val,
        $crate::result::Result::Err(..) => $err
    })
}

The most frequent places where it is necessary for me are:

loop { // or for
    // ...
    try!(foo() => break)
    // ...
}

And:

loop { // or for
   // ...
   try!(foo() => continue)
   // ...
}

You can even write something like this:

    try!(foo() => return DEFAULT_VALUE)

This would be synonymous with unwrap_or_else call:

    try!(foo() => bar())

The code which is proposed now will be fully working too:

    try!(foo() => return)
@hauleth

This comment has been minimized.

Copy link
Author

hauleth commented Dec 8, 2015

@defuz I was thinking about it. However I think that explicit match would be better as it would disallow you to ignore error value which IMHO is bad.

@ticki

This comment has been minimized.

Copy link
Contributor

ticki commented Dec 8, 2015

Essentially, the underlying issue is really the missing ability to define type constraints to expressions in macro definition. Being able to do that, one would be able to define a macro, that act differently from Option and Result.

This can be done in a non-breaking manner along with the new macro definition syntax.

@withoutboats

This comment has been minimized.

Copy link
Contributor

withoutboats commented Dec 8, 2015

@ticki macro expansion occurs at an early stage of compilation in which expressions are not typed. A feature that could be useful here and other places though is the ability to define some kind of polymorphic destructuring.

@ticki

This comment has been minimized.

Copy link
Contributor

ticki commented Dec 8, 2015

@withoutboats Makes sense. I wonder what implications it would have to expand certain macros (with type constraints) later on.

@defuz

This comment has been minimized.

Copy link

defuz commented Dec 8, 2015

@hauleth What is the principal difference between try!(... => return) and try!(... => break)? Both of them allow you to ignore error value.

I think that the basic idea of this proposal is allowed to perform control flow statements (return, break and continue) when Result::Err(..) occurs. Today, there is no way to do this without the explicit match.

Perhaps allowed to do try!(... => bar()) is not the best idea, I think this RFCS should be at least equivalent for return, break and continue.

Also, I don't see any reason why a possible return value should be only ().

@hauleth

This comment has been minimized.

Copy link
Author

hauleth commented Dec 8, 2015

@defuz I don't really think that this proposal should land. I just think that this should be mooted by community if it is desired.

It could be nice addition to #1394, but I think that it should be restricted somehow (i.e. 2 variants for each of return, break and continue).

@BurntSushi

This comment has been minimized.

Copy link
Member

BurntSushi commented Jan 6, 2016

I don't think it's wise to bake a feature into the try! macro that is motivated by dropping error values.

@hauleth

This comment has been minimized.

Copy link
Author

hauleth commented Jan 8, 2016

It isn't wise. It is just discussion about how many people are for or against it.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 11, 2016

🔔 This RFC is now entering its week-long final comment period 🔔

The libs team feels that we are likely to not merge this for now in light of trait-based exception handling having recently landed. We're likely to change try! to simply become ? in which case this may mesh more nicely with catch { ... } as proposed there.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 18, 2016

The libs team discussed this RFC during triage yesterday and the conclusion was to close (in the face of trait-based exception handling). Thanks again though for the RFC @hauleth!

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.