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.

Sign up for GitHub

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

Jump to bottom

Improve assert_eq! and assert_ne! #79100

Merged
merged 6 commits into from
Feb 21, 2021
Merged

Improve assert_eq! and assert_ne! #79100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bbad2b2
Improve assert_eq! and assert_ne!
a1phyr Nov 16, 2020
52197d3
Fix UI tests and merge `assert_eq` and `assert_ne` internal functions
a1phyr Nov 16, 2020
f138e26
Apply suggestion
a1phyr Dec 9, 2020
546d062
Apply suggestions
a1phyr Dec 16, 2020
a357d86
Hide internals items in documentation
a1phyr Jan 16, 2021
7333759
Fix run-make-fulldeps test
a1phyr Feb 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 11 additions & 15 deletions library/core/src/macros/mod.rs
View file
Open in desktop
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
View file
Open in desktop
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 {
a1phyr marked this conversation as resolved.
Show resolved Hide resolved
Eq,
Ne,
}

/// Internal function for `assert_eq!` and `assert_ne!` macros
#[cold]
#[track_caller]
#[doc(hidden)]
pub fn assert_failed<T, U>(
a1phyr marked this conversation as resolved.
Show resolved Hide resolved
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: `{:?}: {}`"#,
Copy link

@ghost ghost Mar 13, 2021 •
edited by ghost
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the change that moves : {} inside the ` quotes intentional?

The panic message now looks suboptimal to me: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=0cbe6c5669d0b7458c6d9f67a3ea7518

Nightly
 right: `3: 1 + 1 should definitely be 3`', src/main.rs:2:5
Stable
 right: `3`: 1 + 1 should definitely be 3', src/main.rs:2:5

(In case it was unintentional, I have a patch.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hyd-dev oh, good catch! Can you send that patch as a PR? (Feel free to assign me with r? @m-ou-se.)

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