Skip to content

Commit

Permalink
Auto merge of #79100 - a1phyr:better_assert_eq, r=m-ou-se
Browse files Browse the repository at this point in the history
Improve assert_eq! and assert_ne!

This PR improves `assert_eq!` and `assert_ne!` by moving the panicking code in an external function.

It does not change the fast path, but the move of the formatting in the cold path (the panic) may have a positive effect on in instruction cache use and with inlining.

Moreover, the use of trait objects instead of generic may improve compile times for `assert_eq!`-heavy code.

Godbolt link: ~~https://rust.godbolt.org/z/TYa9MT~~ \
Updated: https://rust.godbolt.org/z/bzE84x
  • Loading branch information
bors committed Feb 21, 2021
2 parents a31c162 + 7333759 commit ed58a2b
Show file tree
Hide file tree
Showing 15 changed files with 292 additions and 831 deletions.
26 changes: 11 additions & 15 deletions library/core/src/macros/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,32 +53,30 @@ macro_rules! panic {
/// ```
#[macro_export]
#[stable(feature = "rust1", since = "1.0.0")]
#[allow_internal_unstable(core_panic)]
macro_rules! assert_eq {
($left:expr, $right:expr $(,)?) => ({
match (&$left, &$right) {
(left_val, right_val) => {
if !(*left_val == *right_val) {
let kind = $crate::panicking::AssertKind::Eq;
// The reborrows below are intentional. Without them, the stack slot for the
// borrow is initialized even before the values are compared, leading to a
// noticeable slow down.
$crate::panic!(r#"assertion failed: `(left == right)`
left: `{:?}`,
right: `{:?}`"#, &*left_val, &*right_val)
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::None);
}
}
}
});
($left:expr, $right:expr, $($arg:tt)+) => ({
match (&($left), &($right)) {
match (&$left, &$right) {
(left_val, right_val) => {
if !(*left_val == *right_val) {
let kind = $crate::panicking::AssertKind::Eq;
// The reborrows below are intentional. Without them, the stack slot for the
// borrow is initialized even before the values are compared, leading to a
// noticeable slow down.
$crate::panic!(r#"assertion failed: `(left == right)`
left: `{:?}`,
right: `{:?}`: {}"#, &*left_val, &*right_val,
$crate::format_args!($($arg)+))
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::Some($crate::format_args!($($arg)+)));
}
}
}
Expand All @@ -104,17 +102,17 @@ macro_rules! assert_eq {
/// ```
#[macro_export]
#[stable(feature = "assert_ne", since = "1.13.0")]
#[allow_internal_unstable(core_panic)]
macro_rules! assert_ne {
($left:expr, $right:expr $(,)?) => ({
match (&$left, &$right) {
(left_val, right_val) => {
if *left_val == *right_val {
let kind = $crate::panicking::AssertKind::Ne;
// The reborrows below are intentional. Without them, the stack slot for the
// borrow is initialized even before the values are compared, leading to a
// noticeable slow down.
$crate::panic!(r#"assertion failed: `(left != right)`
left: `{:?}`,
right: `{:?}`"#, &*left_val, &*right_val)
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::None);
}
}
}
Expand All @@ -123,13 +121,11 @@ macro_rules! assert_ne {
match (&($left), &($right)) {
(left_val, right_val) => {
if *left_val == *right_val {
let kind = $crate::panicking::AssertKind::Ne;
// The reborrows below are intentional. Without them, the stack slot for the
// borrow is initialized even before the values are compared, leading to a
// noticeable slow down.
$crate::panic!(r#"assertion failed: `(left != right)`
left: `{:?}`,
right: `{:?}`: {}"#, &*left_val, &*right_val,
$crate::format_args!($($arg)+))
$crate::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::Some($crate::format_args!($($arg)+)));
}
}
}
Expand Down
51 changes: 51 additions & 0 deletions library/core/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,54 @@ pub fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! {
// SAFETY: `panic_impl` is defined in safe Rust code and thus is safe to call.
unsafe { panic_impl(&pi) }
}

#[derive(Debug)]
#[doc(hidden)]
pub enum AssertKind {
Eq,
Ne,
}

/// Internal function for `assert_eq!` and `assert_ne!` macros
#[cold]
#[track_caller]
#[doc(hidden)]
pub fn assert_failed<T, U>(
kind: AssertKind,
left: &T,
right: &U,
args: Option<fmt::Arguments<'_>>,
) -> !
where
T: fmt::Debug + ?Sized,
U: fmt::Debug + ?Sized,
{
#[track_caller]
fn inner(
kind: AssertKind,
left: &dyn fmt::Debug,
right: &dyn fmt::Debug,
args: Option<fmt::Arguments<'_>>,
) -> ! {
let op = match kind {
AssertKind::Eq => "==",
AssertKind::Ne => "!=",
};

match args {
Some(args) => panic!(
r#"assertion failed: `(left {} right)`
left: `{:?}`,
right: `{:?}: {}`"#,
op, left, right, args
),
None => panic!(
r#"assertion failed: `(left {} right)`
left: `{:?}`,
right: `{:?}`"#,
op, left, right,
),
}
}
inner(kind, &left, &right, args)
}
Loading

0 comments on commit ed58a2b

Please sign in to comment.