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

Rename the if_ok! macro #12037

Closed
alexcrichton opened this issue Feb 5, 2014 · 17 comments
Closed

Rename the if_ok! macro #12037

alexcrichton opened this issue Feb 5, 2014 · 17 comments
Milestone

Comments

@alexcrichton
Copy link
Member

Removing io_error was bad enough, I didn't want to worry too much about naming this macro at the time. The name if_ok!() isn't fantastic, and we should probably rename it.

  • try!() - short, succinct, but can possibly evoke the notion of try/catch exceptions from other languages
  • ok!() - also short and succinct, but may not always be clear what it's doing.
  • if_ok!() - works well for let statements, not always clear what it's doing though.

Some code that I think should be considered when naming this macro:

fn foo() -> io::IoResult<()> {
    if_ok!(some_function());

    let bar = if_ok!(some_function());

    if if_ok!(some_function()) {
        // ...
    }

    match if_ok!(some_function()) {
        // ...
    }
}

Nominating.

@wycats
Copy link
Contributor

wycats commented Feb 5, 2014

I think that try! is fine, given that it evokes try/catch from other languages.

The semantics of try in other languages is "run this code, and it might throw an exception". Without a catch block, that exception is propagated up the stack.

The semantics of the proposed try! here is "run this code, and it might return an error result. The error is propagated up the stack via Result.

I don't think it's important for the semantics to be identical; new Rust users know that Rust doesn't have exceptions, and the analogy is strong enough to put them on the right path.

@wycats
Copy link
Contributor

wycats commented Feb 5, 2014

Why I don't like if_ok!: In general, commonly used APIs should not be multiple words.

Why I don't like ok!: It has no semantic meaning on its own, and therefore users will not have an easy time forming an association with what the code is doing.

@brson
Copy link
Contributor

brson commented Feb 5, 2014

Here's @alexcrichton's example written with try!

fn foo() -> io::IoResult<()> {
    try!(some_function());

    let bar = try!(some_function());

    if try!(some_function()) {
        // ...
    }

    match try!(some_function()) {
        // ...
    }
}

@brson
Copy link
Contributor

brson commented Feb 5, 2014

We could rename task::try to something else to remove confusion. spawn_try, spawn_catch, spawn_result

@wycats
Copy link
Contributor

wycats commented Feb 5, 2014

FWIW: if try! seems like somewhat less common than let var = try!(expr) or match try!(some_expr).

@nikomatsakis
Copy link
Contributor

I think I like try!, though clearly it's not much like try in other languages, in that it surrounds the things whose failures you do not want to catch, rather than those you do. But I still think it's in the same general spirit. Certainly feels better than ok! and if_ok!.

@sfackler
Copy link
Member

sfackler commented Feb 5, 2014

or_return! is another possibility, though it's a bit long and would read better as the hypothetical r.read(buf).or_return!() instead of or_return!(r.read(buf)).

@kud1ing
Copy link

kud1ing commented Feb 5, 2014

The example with check:

fn foo() -> io::IoResult<()> {
    check!(some_function());

    let bar = check!(some_function());

    if check!(some_function()) {
        // ...
    }

    match check!(some_function()) {
        // ...
    }
}

@lilyball
Copy link
Contributor

lilyball commented Feb 5, 2014

My biggest complaint with try!() is that, when encountering it in code for the first time, it looks like it's going to catch task failure.

Of the 3, I think ok!() is the best, but I don't think it's particularly good in the absolute sense. Maybe just result!()?

fn foo() -> io::IoResult<()> {
    result!(some_function());

    let bar = result!(some_function());

    if result!(some_function()) {
        // ...
    }

    match result!(some_function()) {
        // ...
    }
}

It's not great either, but it doesn't have any preexisting semantic baggage, and for let and match statements it reads like "let bar be the result of some_function()". It also makes it obvious from the name that it has to do with Results.

@pnkfelix
Copy link
Member

pnkfelix commented Feb 6, 2014

.

@pnkfelix pnkfelix added this to the 1.0 milestone Feb 6, 2014
@adrientetar
Copy link
Contributor

check! sounds good.

@liigo
Copy link
Contributor

liigo commented Feb 9, 2014

result!
I mean:

let r: Result = ...
let x = r!; // the postfix operator ! tries unwrap value out of Result or
Option.

2014年2月9日 上午1:29于 "Adrien Tétar" notifications@github.com写道:

check! sounds good.


Reply to this email directly or view it on GitHubhttps://github.com//issues/12037#issuecomment-34549879
.

@wycats
Copy link
Contributor

wycats commented Feb 9, 2014

The goals that this macro has are twofold:

  • Indicate that the expression passed to the macro may be an error
  • Provide a hint that the return value is the value of the successful expression (or at least, not hint to the contrary).

The reason I like try! is that it balances those two needs better than the other options on offer:

  • While try! may have an association with a similar feature in other languages, that very association makes it clear that the expression passed to the macro may fail. In Rust, errors are indicated by a Result type.
  • let file = try!(open(filename)) indicates that file is the value of the successful open(filename) expression as well if not better than the other options presented.

My concerns with the other options:

  • ok!(open(filename)) doesn't really read with any obvious semantics. Not only will readers be forced to look up the semantics in a manual, but I believe that it will become hard for Rust users to form an association between ok!(expr) and the semantics we have in mind here.
  • check!(open(filename)) clearly illustrates that the value is checked, but has at least a slight hint against the correct meaning of the value of the check! expression. It might easily be misconstrued to mean something similar to unwrap() or have a bool value.
  • result!(open(filename)) is somewhat nonsensical because result is a noun, not a verb. It doesn't offer any hints about what happens to the expression or what the return value is.

To me, try! is still the best option I've seen, because it's a nice, short, one-word verb that I think users will easily be able to quickly learn and remember the meaning of.

@hatahet
Copy link
Contributor

hatahet commented Feb 11, 2014

try! is the most appealing out of what has been proposed so far.

@kud1ing
Copy link

kud1ing commented Feb 11, 2014

The macro could be explained like in "call function X, check the result value and either on success use the value in place or return from the calling function". try! to me does not convey the checking and the returning part.

Try and then what? In case of an error:

  • end the program?
  • continue anyway?
  • print "how sad"?
  • ...

@esummers
Copy link

I like if_ok! because it resembles the pattern you would have matched against. How about if_none! if the error condition is wrapped in Option?

Support for monads may be an interesting way to solve this too.

Eric

try! is the most appealing out of what has been proposed so far.


Reply to this email directly or view it on GitHub.

@brson
Copy link
Contributor

brson commented Feb 18, 2014

Let's go with try!.

bors added a commit that referenced this issue Feb 21, 2014
This "bubble up an error" macro was originally named if_ok! in order to get it
landed, but after the fact it was discovered that this name is not exactly
desirable.

The name `if_ok!` isn't immediately clear that is has much to do with error
handling, and it doesn't look fantastic in all contexts (if if_ok!(...) {}). In
general, the agreed opinion about `if_ok!` is that is came in as subpar.

The name `try!` is more invocative of error handling, it's shorter by 2 letters,
and it looks fitting in almost all circumstances. One concern about the word
`try!` is that it's too invocative of exceptions, but the belief is that this
will be overcome with documentation and examples.

Close #12037
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
fix: Remove `rust-analyzer.inlayHints.enable` and set language scope

Closes rust-lang#12036
CC rust-lang/rust-analyzer#12027 (comment)

The key was left there by mistake in rust-lang#12006.

Setting the configuration scope only works if you already have it created, which is fine, but unfortunately not quite discoverable.
flip1995 pushed a commit to flip1995/rust that referenced this issue Jan 11, 2024
add external macro checks to `iter_without_into_iter` and `into_iter_without_iter`

Fixes rust-lang#12037

I think it's useful to still lint on local macros, since the user should still be able to add another impl with the `IntoIterator` or `iter` method. I think it's also fairly common to write a macro for generating many impls (e.g. for many similar types), so it'd be nice if we can continue linting in those cases.
For that reason I went with `in_external_macro`.

I also added a test for `#[allow]`ing the lint while I was at it.

changelog: [`iter_without_into_iter`]: don't lint if the `iter` method is defined in an external macro
changelog: [`into_iter_without_iter`]: don't lint if the `IntoIterator` impl is defined in an external macro
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.