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 Option::unwrap_unchecked #48278

Closed
joshlf opened this issue Feb 16, 2018 · 4 comments · Fixed by #80876
Closed

Add Option::unwrap_unchecked #48278

joshlf opened this issue Feb 16, 2018 · 4 comments · Fixed by #80876
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@joshlf
Copy link
Contributor

joshlf commented Feb 16, 2018

Now that NonNull has been added and Option<NonNull<T>> is preferred over *mut T, if you have code that used to know that a *mut T was non-null and dereferenced it, you now know that your Option<NonNull<T>> is Some and can call .unwrap() on it. However, .unwrap() can incur runtime overhead that dereferencing *mut T never would. Since a lot of performance-critical code is written using raw pointers (and now, NonNull), this runtime overhead may be worth worrying about.

I propose, for these cases, to introduce an unsafe unwrap_unchecked method to Option that returns a T by simply assuming that the Option is currently Some(T).

@kennytm kennytm added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Feb 16, 2018
@kennytm
Copy link
Member

kennytm commented Feb 16, 2018

There is already an external package providing this function. https://crates.io/crates/unsafe_unwrap

@joshlf
Copy link
Contributor Author

joshlf commented Feb 16, 2018

Ah good to know. I still think it could be useful in the stdlib, but it's definitely less of a concern with that crate.

@aleksmelnikov
Copy link

My compare of default unwrap() and unsafe_unwrap():

$ git clone https://github.com/nvzqz/unsafe-unwrap-rs.git
$ cd unsafe-unwrap-rs
$ cargo build
$ cargo bench

The bench output:

test benches::bench_normal_unwrap_1000 ... bench:       2,112 ns/iter (+/- 709)
test benches::bench_unsafe_unwrap_1000 ... bench:         814 ns/iter (+/- 89)

Code compare:

// default unwrap()
impl<T> Option<T> {
    pub fn unwrap(self) -> T {
        match self {
            Some(val) => val,
            None => panic!("called `Option::unwrap()` on a `None` value"),
        }
  
    }
}
// unsafe_unwrap()
impl<T> UnsafeUnwrap<T> for Option<T> {
    #[inline]
    unsafe fn unsafe_unwrap(self) -> T {
     if let Some(x) = self { x } else { unreachable() }
    }
}

I tried to change code of unsafe_unwrap():

impl<T> UnsafeUnwrap<T> for Option<T> {
    #[inline]
    unsafe fn unsafe_unwrap(self) -> T {
     //if let Some(x) = self { x } else { unreachable() }
     if let Some(x) = self { x } else { panic!("PANIC") }
    }
}

New bench output:

test benches::bench_normal_unwrap_1000 ... bench:       2,111 ns/iter (+/- 56)
test benches::bench_unsafe_unwrap_1000 ... bench:       2,110 ns/iter (+/- 13)

So, panic! macro slows down default unwrap() in ~2.5 times!

JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 26, 2021
…d, r=m-ou-se

Add `unwrap_unchecked()` methods for `Option` and `Result`

In particular:
  - `unwrap_unchecked()` for `Option`.
  - `unwrap_unchecked()` and `unwrap_err_unchecked()` for `Result`.

These complement other `*_unchecked()` methods in `core` etc.

Currently there are a couple of places it may be used inside rustc (`LinkedList`, `BTree`). It is also easy to find other repositories with similar functionality.

Fixes rust-lang#48278.
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 26, 2021
…d, r=m-ou-se

Add `unwrap_unchecked()` methods for `Option` and `Result`

In particular:
  - `unwrap_unchecked()` for `Option`.
  - `unwrap_unchecked()` and `unwrap_err_unchecked()` for `Result`.

These complement other `*_unchecked()` methods in `core` etc.

Currently there are a couple of places it may be used inside rustc (`LinkedList`, `BTree`). It is also easy to find other repositories with similar functionality.

Fixes rust-lang#48278.
@bors bors closed this as completed in 679f6f3 Jan 26, 2021
@ojeda
Copy link
Contributor

ojeda commented Jan 28, 2021

Available in nightly now. For Result too.

zaharidichev pushed a commit to zaharidichev/rust that referenced this issue Mar 15, 2021
In particular:
  - `unwrap_unchecked()` for `Option`.
  - `unwrap_unchecked()` and `unwrap_err_unchecked()` for `Result`.

These complement other `*_unchecked()` methods in `core` etc.

Currently there are a couple of places it may be used inside rustc
(`LinkedList`, `BTree`). It is also easy to find other repositories
with similar functionality.

Fixes rust-lang#48278.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants