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

assert_eq! message format (take 2) #111030

Closed
wants to merge 16 commits into from
Closed

assert_eq! message format (take 2) #111030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
a319d02
optimize panic message format in assert_eq macro
MakitaToki Feb 15, 2022
c95dd49
align colons in assert_failed_inner
MakitaToki Feb 16, 2022
9a3aca6
change message format in 'assert_failed_inner' function
MakitaToki Feb 19, 2022
af31311
delete trailing whitespaces
Feb 19, 2022
cdfe6b5
stringify original tokens in related macros
MakitaToki Feb 28, 2022
8348b5a
clean up output format
MakitaToki Mar 6, 2022
68d25de
Address review comments
nyurik Apr 30, 2023
55d21c1
handle multiline panics, some tests
nyurik May 1, 2023
025f22f
Output cleanup, fix tests, handle match assert
nyurik May 1, 2023
9938a7e
mir bless, fmt fix
nyurik May 1, 2023
3a39a74
Add a var test, make all tests bless-able
nyurik May 1, 2023
d2fcd41
Try to fix clippy lints, reorder new args to the end
nyurik May 1, 2023
b0549f1
remove extra brackets, better var test
nyurik May 1, 2023
a2d6f73
fix mir tests
nyurik May 1, 2023
dd6b37a
try to improve compilation performance
nyurik May 1, 2023
e785f8b
remove unused AssertKind
nyurik May 1, 2023
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
24 changes: 14 additions & 10 deletions library/core/src/macros/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ macro_rules! assert_eq {
match (&$left, &$right) {
(left_val, right_val) => {
if !(*left_val == *right_val) {
let kind = $crate::panicking::AssertKind::Eq;
let assert = $crate::concat!($crate::stringify!($left), " == ", $crate::stringify!($right));
// 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::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::None);
$crate::panicking::assert_failed(assert, &*left_val, &*right_val, $crate::option::Option::None);
}
}
}
Expand All @@ -51,11 +51,11 @@ macro_rules! assert_eq {
match (&$left, &$right) {
(left_val, right_val) => {
if !(*left_val == *right_val) {
let kind = $crate::panicking::AssertKind::Eq;
let assert = $crate::concat!($crate::stringify!($left), " == ", $crate::stringify!($right));
// 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::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::Some($crate::format_args!($($arg)+)));
$crate::panicking::assert_failed(assert, &*left_val, &*right_val, $crate::option::Option::Some($crate::format_args!($($arg)+)));
}
}
}
Expand Down Expand Up @@ -88,11 +88,11 @@ macro_rules! assert_ne {
match (&$left, &$right) {
(left_val, right_val) => {
if *left_val == *right_val {
let kind = $crate::panicking::AssertKind::Ne;
let assert = $crate::concat!($crate::stringify!($left), " != ", $crate::stringify!($right));
// 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::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::None);
$crate::panicking::assert_failed(assert, &*left_val, &*right_val, $crate::option::Option::None);
}
}
}
Expand All @@ -101,11 +101,11 @@ macro_rules! assert_ne {
match (&($left), &($right)) {
(left_val, right_val) => {
if *left_val == *right_val {
let kind = $crate::panicking::AssertKind::Ne;
let assert = $crate::concat!($crate::stringify!($left), " != ", $crate::stringify!($right));
// 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::panicking::assert_failed(kind, &*left_val, &*right_val, $crate::option::Option::Some($crate::format_args!($($arg)+)));
$crate::panicking::assert_failed(assert, &*left_val, &*right_val, $crate::option::Option::Some($crate::format_args!($($arg)+)));
}
}
}
Expand Down Expand Up @@ -146,10 +146,12 @@ pub macro assert_matches {
match $left {
$( $pattern )|+ $( if $guard )? => {}
ref left_val => {
let assert = $crate::concat!($crate::stringify!($left), " matches ", $crate::stringify!($($pattern)|+ $(if $guard)?));
$crate::panicking::assert_matches_failed(
left_val,
$crate::stringify!($($pattern)|+ $(if $guard)?),
$crate::option::Option::None
$crate::option::Option::None,
assert,
);
}
}
Expand All @@ -158,10 +160,12 @@ pub macro assert_matches {
match $left {
$( $pattern )|+ $( if $guard )? => {}
ref left_val => {
let assert = $crate::concat!($crate::stringify!($left), " matches ", $crate::stringify!($($pattern)|+ $(if $guard)?));
$crate::panicking::assert_matches_failed(
left_val,
$crate::stringify!($($pattern)|+ $(if $guard)?),
$crate::option::Option::Some($crate::format_args!($($arg)+))
$crate::option::Option::Some($crate::format_args!($($arg)+)),
assert,
);
}
}
Expand Down
38 changes: 12 additions & 26 deletions library/core/src/panicking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,21 +202,13 @@ pub const fn const_panic_fmt(fmt: fmt::Arguments<'_>) -> ! {
}
}

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

/// Internal function for `assert_eq!` and `assert_ne!` macros
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller]
#[doc(hidden)]
pub fn assert_failed<T, U>(
kind: AssertKind,
assert_msg: &'static str,
left: &T,
right: &U,
args: Option<fmt::Arguments<'_>>,
Expand All @@ -225,7 +217,7 @@ where
T: fmt::Debug + ?Sized,
U: fmt::Debug + ?Sized,
{
assert_failed_inner(kind, &left, &right, args)
assert_failed_inner(assert_msg, &left, &right, args)
}

/// Internal function for `assert_match!`
Expand All @@ -237,6 +229,7 @@ pub fn assert_matches_failed<T: fmt::Debug + ?Sized>(
left: &T,
right: &str,
args: Option<fmt::Arguments<'_>>,
assert_msg: &'static str,
) -> ! {
// The pattern is a string so it can be displayed directly.
struct Pattern<'a>(&'a str);
Expand All @@ -245,37 +238,30 @@ pub fn assert_matches_failed<T: fmt::Debug + ?Sized>(
f.write_str(self.0)
}
}
assert_failed_inner(AssertKind::Match, &left, &Pattern(right), args);
assert_failed_inner(assert_msg, &left, &Pattern(right), args);
}

/// Non-generic version of the above functions, to avoid code bloat.
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller]
fn assert_failed_inner(
kind: AssertKind,
assert_msg: &'static str,
left: &dyn fmt::Debug,
right: &dyn fmt::Debug,
args: Option<fmt::Arguments<'_>>,
) -> ! {
let op = match kind {
AssertKind::Eq => "==",
AssertKind::Ne => "!=",
AssertKind::Match => "matches",
};

match args {
Some(args) => panic!(
r#"assertion failed: `(left {} right)`
left: `{:?}`,
right: `{:?}`: {}"#,
op, left, right, args
r#"assertion failed: `{assert_msg}`
error: {args}
left: `{left:?}`
right: `{right:?}`"#
),
None => panic!(
r#"assertion failed: `(left {} right)`
left: `{:?}`,
right: `{:?}`"#,
op, left, right,
r#"assertion failed: `{assert_msg}`
left: `{left:?}`
right: `{right:?}`"#
),
}
}
6 changes: 5 additions & 1 deletion library/std/src/panicking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,11 @@ fn default_hook(info: &PanicInfo<'_>) {
let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");

let write = |err: &mut dyn crate::io::Write| {
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");
if msg.contains('\n') {
let _ = writeln!(err, "thread '{name}' panicked at {location}:\n{msg}");
} else {
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");
}

static FIRST_PANIC: AtomicBool = AtomicBool::new(true);

Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/clippy_utils/src/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ impl<'a> PanicExpn<'a> {
"assert_failed" => {
// It should have 4 arguments in total (we already matched with the first argument,
// so we're just checking for 3)
// Since Rust 1.70, `assert_failed` has two additional args.
if rest.len() != 3 {
return None;
}
Expand Down
30 changes: 18 additions & 12 deletions tests/mir-opt/issue_99325.main.built.after.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fn main() -> () {
let mut _13: &&[u8; 4]; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _14: !; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _16: !; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _17: core::panicking::AssertKind; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _17: &str; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _18: &&[u8]; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _19: &&[u8]; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _20: &&[u8; 4]; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
Expand All @@ -41,7 +41,7 @@ fn main() -> () {
let mut _34: &&[u8; 4]; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _35: !; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _37: !; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _38: core::panicking::AssertKind; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _38: &str; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _39: &&[u8]; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _40: &&[u8]; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _41: &&[u8; 4]; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
Expand All @@ -50,17 +50,17 @@ fn main() -> () {
scope 1 {
debug left_val => _8; // in scope 1 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
debug right_val => _9; // in scope 1 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _15: core::panicking::AssertKind; // in scope 1 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _15: &str; // in scope 1 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
scope 2 {
debug kind => _15; // in scope 2 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
debug assert => _15; // in scope 2 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
}
}
scope 3 {
debug left_val => _29; // in scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
debug right_val => _30; // in scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _36: core::panicking::AssertKind; // in scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _36: &str; // in scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
scope 4 {
debug kind => _36; // in scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
debug assert => _36; // in scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
}
}

Expand Down Expand Up @@ -114,11 +114,14 @@ fn main() -> () {

bb3: {
StorageLive(_15); // scope 1 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_15 = core::panicking::AssertKind::Eq; // scope 1 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_15 = const "function_with_bytes::<b\"AAAA\">() == &[0x41, 0x41, 0x41, 0x41]"; // scope 1 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL
// + literal: Const { ty: &str, val: Value(Slice(..)) }
FakeRead(ForLet(None), _15); // scope 1 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_16); // scope 2 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_17); // scope 2 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_17 = move _15; // scope 2 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_17 = &(*_15); // scope 2 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_18); // scope 2 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_19); // scope 2 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_19 = &(*_8); // scope 2 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
Expand All @@ -132,7 +135,7 @@ fn main() -> () {
_16 = core::panicking::assert_failed::<&[u8], &[u8; 4]>(move _17, move _18, move _20, move _22) -> bb19; // scope 2 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL
// + literal: Const { ty: for<'a, 'b, 'c> fn(core::panicking::AssertKind, &'a &[u8], &'b &[u8; 4], Option<Arguments<'c>>) -> ! {core::panicking::assert_failed::<&[u8], &[u8; 4]>}, val: Value(<ZST>) }
// + literal: Const { ty: for<'a, 'b, 'c> fn(&'static str, &'a &[u8], &'b &[u8; 4], Option<Arguments<'c>>) -> ! {core::panicking::assert_failed::<&[u8], &[u8; 4]>}, val: Value(<ZST>) }
}

bb4: {
Expand Down Expand Up @@ -223,11 +226,14 @@ fn main() -> () {

bb12: {
StorageLive(_36); // scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_36 = core::panicking::AssertKind::Eq; // scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_36 = const "function_with_bytes::<{ &[0x41, 0x41, 0x41, 0x41] }>() == b\"AAAA\""; // scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL
// + literal: Const { ty: &str, val: Value(Slice(..)) }
FakeRead(ForLet(None), _36); // scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_37); // scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_38); // scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_38 = move _36; // scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_38 = &(*_36); // scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_39); // scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_40); // scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_40 = &(*_29); // scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
Expand All @@ -241,7 +247,7 @@ fn main() -> () {
_37 = core::panicking::assert_failed::<&[u8], &[u8; 4]>(move _38, move _39, move _41, move _43) -> bb19; // scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL
// + literal: Const { ty: for<'a, 'b, 'c> fn(core::panicking::AssertKind, &'a &[u8], &'b &[u8; 4], Option<Arguments<'c>>) -> ! {core::panicking::assert_failed::<&[u8], &[u8; 4]>}, val: Value(<ZST>) }
// + literal: Const { ty: for<'a, 'b, 'c> fn(&'static str, &'a &[u8], &'b &[u8; 4], Option<Arguments<'c>>) -> ! {core::panicking::assert_failed::<&[u8], &[u8; 4]>}, val: Value(<ZST>) }
}

bb13: {
Expand Down
16 changes: 10 additions & 6 deletions tests/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fn array_casts() -> () {
let mut _25: usize; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _26: !; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _28: !; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _29: core::panicking::AssertKind; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _29: &str; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _30: &usize; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _31: &usize; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let mut _32: &usize; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
Expand All @@ -51,9 +51,9 @@ fn array_casts() -> () {
scope 7 {
debug left_val => _20; // in scope 7 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
debug right_val => _21; // in scope 7 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _27: core::panicking::AssertKind; // in scope 7 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
let _27: &str; // in scope 7 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
scope 8 {
debug kind => _27; // in scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
debug assert => _27; // in scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
}
}
}
Expand Down Expand Up @@ -148,10 +148,14 @@ fn array_casts() -> () {

bb3: {
StorageLive(_27); // scope 7 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_27 = core::panicking::AssertKind::Eq; // scope 7 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_27 = const "unsafe { *p.add(1) } == 1"; // scope 7 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL
// + literal: Const { ty: &str, val: Value(Slice(..)) }
Retag(_27); // scope 7 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_28); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_29); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_29 = move _27; // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_29 = &(*_27); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_30); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageLive(_31); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_31 = &(*_20); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
Expand All @@ -166,7 +170,7 @@ fn array_casts() -> () {
_28 = core::panicking::assert_failed::<usize, usize>(move _29, move _30, move _32, move _34); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL
// + literal: Const { ty: for<'a, 'b, 'c> fn(core::panicking::AssertKind, &'a usize, &'b usize, Option<Arguments<'c>>) -> ! {core::panicking::assert_failed::<usize, usize>}, val: Value(<ZST>) }
// + literal: Const { ty: for<'a, 'b, 'c> fn(&'static str, &'a usize, &'b usize, Option<Arguments<'c>>) -> ! {core::panicking::assert_failed::<usize, usize>}, val: Value(<ZST>) }
}

bb4: {
Expand Down
4 changes: 1 addition & 3 deletions tests/ui/macros/assert-eq-macro-msg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// run-fail
// error-pattern:panicked at 'assertion failed: `(left == right)`
// error-pattern: left: `2`
// error-pattern:right: `3`: 1 + 1 definitely should be 3'
// check-run-results
// ignore-emscripten no processes

fn main() {
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/macros/assert-eq-macro-msg.run.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
thread 'main' panicked at $DIR/assert-eq-macro-msg.rs:6:5:
assertion failed: `1 + 1 == 3`
error: 1 + 1 definitely should be 3
left: `2`
right: `3`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
4 changes: 1 addition & 3 deletions tests/ui/macros/assert-eq-macro-panic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// run-fail
// error-pattern:assertion failed: `(left == right)`
// error-pattern: left: `14`
// error-pattern:right: `15`
// check-run-results
// ignore-emscripten no processes

fn main() {
Expand Down
Loading