Skip to content

Commit

Permalink
Auto merge of #119878 - scottmcm:inline-always-unwrap, r=<try>
Browse files Browse the repository at this point in the history
Tune the inlinability of `unwrap`

Fixes #115463
cc `@thomcc`

This tweaks `unwrap` on `Option` & `Result` to be two parts:
- `#[inline(always)]` for checking the discriminant
- `#[cold]` for actually panicking

The idea here is that checking the discriminant on a `Result` or `Option` should always be trivial enough to be worth inlining, even in `opt-level=z`, especially compared to passing it to a function.

As seen in the issue and codegen test, this will hopefully help particularly for things like `.try_into().unwrap()`s that are actually infallible, but in a way that's only visible with the inlining.
  • Loading branch information
bors committed Jan 12, 2024
2 parents bfd799f + b996061 commit b03798d
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 5 deletions.
12 changes: 10 additions & 2 deletions library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -921,14 +921,14 @@ impl<T> Option<T> {
/// let x: Option<&str> = None;
/// assert_eq!(x.unwrap(), "air"); // fails
/// ```
#[inline]
#[inline(always)]
#[track_caller]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_option", issue = "67441")]
pub const fn unwrap(self) -> T {
match self {
Some(val) => val,
None => panic("called `Option::unwrap()` on a `None` value"),
None => unwrap_failed(),
}
}

Expand Down Expand Up @@ -1970,6 +1970,14 @@ impl<T, E> Option<Result<T, E>> {
}
}

#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller]
const fn unwrap_failed() -> ! {
panic("called `Option::unwrap()` on a `None` value")
}

// This is a separate function to reduce the code size of .expect() itself.
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,7 @@ impl<T, E> Result<T, E> {
/// let x: Result<u32, &str> = Err("emergency failure");
/// x.unwrap(); // panics with `emergency failure`
/// ```
#[inline]
#[inline(always)]
#[track_caller]
#[stable(feature = "rust1", since = "1.0.0")]
pub fn unwrap(self) -> T
Expand Down
4 changes: 2 additions & 2 deletions tests/codegen/debuginfo-inline-callsite-location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
// can correctly merge the debug info if it merges the inlined code (e.g., for merging of tail
// calls to panic.

// CHECK: tail call void @_ZN4core9panicking5panic17h{{([0-9a-z]{16})}}E
// CHECK: tail call void @_ZN4core6option13unwrap_failed17h{{([0-9a-z]{16})}}E
// CHECK-SAME: !dbg ![[#first_dbg:]]
// CHECK: tail call void @_ZN4core9panicking5panic17h{{([0-9a-z]{16})}}E
// CHECK: tail call void @_ZN4core6option13unwrap_failed17h{{([0-9a-z]{16})}}E
// CHECK-SAME: !dbg ![[#second_dbg:]]

// CHECK-DAG: ![[#func_dbg:]] = distinct !DISubprogram(name: "unwrap<i32>"
Expand Down
26 changes: 26 additions & 0 deletions tests/codegen/infallible-unwrap-in-opt-z.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// compile-flags: -C opt-level=z --edition=2021
// ignore-debug

#![crate_type = "lib"]

// From <https://github.com/rust-lang/rust/issues/115463>

// CHECK-LABEL: @read_up_to_8(
#[no_mangle]
pub fn read_up_to_8(buf: &[u8]) -> u64 {
// CHECK-NOT: unwrap_failed
if buf.len() < 4 {
// actual instance has more code.
return 0;
}
let lo = u32::from_le_bytes(buf[..4].try_into().unwrap()) as u64;
let hi = u32::from_le_bytes(buf[buf.len() - 4..][..4].try_into().unwrap()) as u64;
lo | (hi << 8 * (buf.len() as u64 - 4))
}

// CHECK-LABEL: @checking_unwrap_expectation(
#[no_mangle]
pub fn checking_unwrap_expectation(buf: &[u8]) -> &[u8; 4] {
// CHECK: call void @_ZN4core6result13unwrap_failed17h
buf.try_into().unwrap()
}

0 comments on commit b03798d

Please sign in to comment.