Cleaner assert_eq! & assert_ne! panic messages by nyurik · Pull Request #111071 · rust-lang/rust

nyurik · 2023-05-01T16:35:24Z

Note: See #112849 (comment) for the snippet for the Rust 1.73 release blog post.

This PR finishes refactoring of the assert messages per #94005. The panic message format change #112849 used to be part of this PR, but has been factored out and just merged. It might be better to keep both changes in the same release once FCP vote completes.

Modify panic message for assert_eq!, assert_ne!, the currently unstable assert_matches!, as well as the corresponding debug_assert_* macros.

assert_eq!(1 + 1, 3);
assert_eq!(1 + 1, 3, "my custom message value={}!", 42);

Old messages

thread 'main' panicked at $DIR/main.rs:6:5:
assertion failed: `(left == right)`
  left: `2`,
 right: `3`

thread 'main' panicked at $DIR/main.rs:6:5:
assertion failed: `(left == right)`
  left: `2`,
 right: `3`: my custom message value=42!

New messages

thread 'main' panicked at $DIR/main.rs:6:5:
assertion `left == right` failed
  left: 2
 right: 3

thread 'main' panicked at $DIR/main.rs:6:5:
assertion `left == right` failed: my custom message value=42!
  left: 2
 right: 3

History of fixing #94005

improve custom message format in assert_eq macro #94016 was a lengthy PR that was abandoned
assert_eq! message format (take 2) #111030 was similar, but it stringified left and right arguments, and thus caused compile time performance issues, thus closed
Change default panic handler message format. #112849 factored out the two-line formatting of all panic messages

Fixes #94005

r? @m-ou-se

rustbot · 2023-05-01T16:35:32Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

m-ou-se · 2023-05-01T19:19:01Z

@bors try @rust-timer queue

bors · 2023-05-01T19:19:10Z

⌛ Trying commit 42ed1f3a4f29e7518b514818edacfdcebef6f850 with merge 0687ecd68a4be6f12ad402aaf80d542013ee18a6...

bors · 2023-05-01T21:35:53Z

☀️ Try build successful - checks-actions
Build commit: 0687ecd68a4be6f12ad402aaf80d542013ee18a6 (0687ecd68a4be6f12ad402aaf80d542013ee18a6)

rust-timer · 2023-05-02T00:27:13Z

Finished benchmarking commit (0687ecd68a4be6f12ad402aaf80d542013ee18a6): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.9%	[0.8%, 1.0%]	2
Improvements ✅ (primary)	-2.6%	[-3.5%, -2.2%]	4
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.6%	[-3.5%, -2.2%]	4

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 656.548s -> 655.057s (-0.23%)

nyurik · 2023-05-02T00:33:44Z

@m-ou-se well, i guess we can simply merge this, and separately discuss if we want to print left == right vs snippets from the user's code like expr1 == expr2

nyurik · 2023-05-09T21:28:56Z

@m-ou-se friendly ping - anything left to do on this one? Any blockers to merge it?

apiraino · 2023-05-23T14:51:18Z

unless I'm mistaken I think T-compiler can be pulled from the review queue here

@rustbot label -T-compiler

nyurik · 2023-05-24T00:43:23Z

@apiraino agree, this is just a tiny assert_eq macro output formatting. This has been untouched for a while - is there anything blocking this PR from merging? Seems like this is the simplest path forward without any raised concerns.

nyurik · 2023-05-27T19:13:44Z

@yaahc hi, you were initially reviewing the #94016, but that one has gone stale and was closed. This small PR has been open for a while, addresses your comments, and has no performance impact. Should it be reassigned to you perhaps? I don't want to stress @m-ou-se - she already has tons of amazing work on her plate (thx!!!)

cc: @jyn514 (thx for the comments on the original issue!)

m-ou-se · 2023-05-27T19:16:14Z

I'll have time for reviews again next week. (Last few weeks were basically entirely meetings/conferences/etc.)

nyurik · 2023-05-27T19:19:29Z

Thank you @m-ou-se!!! Looking forward to it :)

m-ou-se · 2023-06-05T12:47:44Z

Nominating for team discussion.

This PR changes the way multi-line panic messages are printed by the default hook from

thread '{thread}' panicked at '{message}', {location}

to

thread '{thread}' panicked at {location}:
{message}

Question 1: Do we want this, and if so, do we want to do this only for multi-line messages? It might make sense to do this for all messages, such that it always says "at {location}", but that would probably break a bunch of test suites that match for exact panic output.. (Although they should probably be using a custom panic hook?)

This PR also changes the panic message of assert_eq from:

assertion failed: `(left == right)`
  left: `1`,
 right: `2`: asdf

to

assertion failed: `left == right`
 error: asdf
  left: `1`
 right: `2`

Question 2: Do we want this change for assert_eq? Is it appropriate to call the message error? Is it an improvement over the previous output?

For the bigger picture: the two changes above combined mean that the default output of
assert_eq!(1, 2, "asdf") changes from:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `2`: asdf', src/main.rs:2:5

to:

thread 'main' panicked at src/main.rs:2:5:
assertion failed: `left == right`
 error: asdf
  left: `1`
 right: `2`

nyurik · 2023-08-15T16:10:43Z

@m-ou-se just in case it fails again (hope not!), i think i saw there was a magical bors command to allow me to bors+ it myself for this specific PR? Someone did it once for my other PR a while back, can't recall which. This way i won't bug you every time it fails :)

albertlarsan68 · 2023-08-15T18:39:32Z

There is the bors delegate+ command.

nyurik · 2023-08-15T18:40:47Z

I don't think i can run that command 🤣

bors · 2023-08-15T19:05:27Z

⌛ Testing commit afbb8ca2b2057dfe82c5146c1175f3f686cfd8cf with merge 72b3af3f1cd0b49bb64e2469ead7473244233253...

nyurik · 2023-08-15T20:49:28Z

Here's a magical spell to fix this problem: 🪄 RIDICULOUS! 🪄

I will revert the assertions back to the old style sub-string checks 🤦

@Urgau or @m-ou-se could you delegate this one to me? I will submit a PR shortly

Modify panic message for `assert_eq!`, `assert_ne!`, the currently unstable `assert_matches!`, as well as the corresponding `debug_assert_*` macros. ```rust assert_eq!(1 + 1, 3); assert_eq!(1 + 1, 3, "my custom message value={}!", 42); ``` ```plain thread 'main' panicked at $DIR/main.rs:6:5: assertion failed: `(left == right)` left: `2`, right: `3` ``` ```plain thread 'main' panicked at $DIR/main.rs:6:5: assertion failed: `(left == right)` left: `2`, right: `3`: my custom message value=42! ``` ```plain thread 'main' panicked at $DIR/main.rs:6:5: assertion `left == right` failed left: 2 right: 3 ``` ```plain thread 'main' panicked at $DIR/main.rs:6:5: assertion `left == right` failed: my custom message value=42! left: 2 right: 3 ``` This PR is a simpler subset of the rust-lang#111030, but it does NOT stringify the original left and right source code assert expressions, thus should be faster to compile.

nyurik · 2023-08-15T21:06:10Z

Please re-bors it! Hopefully THIS TIME the 🪄 will work...
Only 72 comments on this PR, no biggie...

m-ou-se · 2023-08-15T22:20:43Z

@bors r+ p=1

bors · 2023-08-15T22:20:45Z

📌 Commit 950e3d9 has been approved by m-ou-se

It is now in the queue for this repository.

bors · 2023-08-15T22:46:00Z

⌛ Testing commit 950e3d9 with merge b531630...

bors · 2023-08-16T00:34:15Z

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing b531630 to master...

nyurik · 2023-08-16T01:03:36Z

🎉 Horray! It only took 548 days (1.5+ years) since I created #94005 on Feb 14th, 2022. Numerous commits. Tens of people. 4 PRs... 100s of files. All to change a few lines of output :)

Special thanks to @m-ou-se for all the support!

rust-timer · 2023-08-16T01:56:51Z

Finished benchmarking commit (b531630): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.6%	[0.5%, 0.6%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.7%	[2.7%, 2.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.7%	[2.7%, 2.7%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 633.84s -> 634.559s (0.11%)
Artifact size: 346.89 MiB -> 346.80 MiB (-0.02%)

m-ou-se · 2023-08-16T07:16:04Z

Congrats @nyurik! ^^

Upstream changes: - "Stabilize thread local cell methods." rust-lang/rust#114689 - "Cleaner assert_eq! & assert_ne! panic messages" rust-lang/rust#111071

Language -------- - [Uplift `clippy::fn_null_check` lint as `useless_ptr_null_checks`.] (rust-lang/rust#111717) - [Make `noop_method_call` warn by default.] (rust-lang/rust#111916) - [Support interpolated block for `try` and `async` in macros.] (rust-lang/rust#112953) - [Make `unconditional_recursion` lint detect recursive drops.] (rust-lang/rust#113902) - [Future compatibility warning for some impls being incorrectly considered not overlapping.] (rust-lang/rust#114023) - [The `invalid_reference_casting` lint is now **deny-by-default** (instead of allow-by-default)] (rust-lang/rust#112431) Compiler -------- - [Write version information in a `.comment` section like GCC/Clang.] (rust-lang/rust#97550) - [Add documentation on v0 symbol mangling.] (rust-lang/rust#97571) - [Stabilize `extern "thiscall"` and `"thiscall-unwind"` ABIs.] (rust-lang/rust#114562) - [Only check outlives goals on impl compared to trait.] (rust-lang/rust#109356) - [Infer type in irrefutable slice patterns with fixed length as array.] (rust-lang/rust#113199) - [Discard default auto trait impls if explicit ones exist.] (rust-lang/rust#113312) - Add several new tier 3 targets: - [`aarch64-unknown-teeos`] (rust-lang/rust#113480) - [`csky-unknown-linux-gnuabiv2`] (rust-lang/rust#113658) - [`riscv64-linux-android`] (rust-lang/rust#112858) - [`riscv64gc-unknown-hermit`] (rust-lang/rust#114004) - [`x86_64-unikraft-linux-musl`] (rust-lang/rust#113411) - [`x86_64-unknown-linux-ohos`] (rust-lang/rust#113061) - [Add `wasm32-wasi-preview1-threads` as a tier 2 target.] (rust-lang/rust#112922) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Add `Read`, `Write` and `Seek` impls for `Arc<File>`.] (rust-lang/rust#94748) - [Merge functionality of `io::Sink` into `io::Empty`.] (rust-lang/rust#98154) - [Implement `RefUnwindSafe` for `Backtrace`] (rust-lang/rust#100455) - [Make `ExitStatus` implement `Default`] (rust-lang/rust#106425) - [`impl SliceIndex<str> for (Bound<usize>, Bound<usize>)`] (rust-lang/rust#111081) - [Change default panic handler message format.] (rust-lang/rust#112849) - [Cleaner `assert_eq!` & `assert_ne!` panic messages.] (rust-lang/rust#111071) - [Correct the (deprecated) Android `stat` struct definitions.] (rust-lang/rust#113130) Stabilized APIs --------------- - [Unsigned `{integer}::div_ceil`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.div_ceil) - [Unsigned `{integer}::next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.next_multiple_of) - [Unsigned `{integer}::checked_next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.checked_next_multiple_of) - [`std::ffi::FromBytesUntilNulError`] (https://doc.rust-lang.org/stable/std/ffi/struct.FromBytesUntilNulError.html) - [`std::os::unix::fs::chown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.chown.html) - [`std::os::unix::fs::fchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.fchown.html) - [`std::os::unix::fs::lfchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.lchown.html) - [`LocalKey::<Cell<T>>::get`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.get) - [`LocalKey::<Cell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set) - [`LocalKey::<Cell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take) - [`LocalKey::<Cell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace) - [`LocalKey::<RefCell<T>>::with_borrow`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow) - [`LocalKey::<RefCell<T>>::with_borrow_mut`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow_mut) - [`LocalKey::<RefCell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set-1) - [`LocalKey::<RefCell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take-1) - [`LocalKey::<RefCell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace-1) These APIs are now stable in const contexts: - [`rc::Weak::new`] (https://doc.rust-lang.org/stable/alloc/rc/struct.Weak.html#method.new) - [`sync::Weak::new`] (https://doc.rust-lang.org/stable/alloc/sync/struct.Weak.html#method.new) - [`NonNull::as_ref`] (https://doc.rust-lang.org/stable/core/ptr/struct.NonNull.html#method.as_ref) Cargo ----- - [Encode URL params correctly for `SourceId` in `Cargo.lock`.] (rust-lang/cargo#12280) - [Bail out an error when using `cargo::` in custom build script.] (rust-lang/cargo#12332) Compatibility Notes ------------------- - [Update the minimum external LLVM to 15.] (rust-lang/rust#114148) - [Check for non-defining uses of return position `impl Trait`.] (rust-lang/rust#112842) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Remove LLVM pointee types, supporting only opaque pointers.] (rust-lang/rust#105545) - [Port PGO/LTO/BOLT optimized build pipeline to Rust.] (rust-lang/rust#112235) - [Replace in-tree `rustc_apfloat` with the new version of the crate.] (rust-lang/rust#113843) - [Update to LLVM 17.] (rust-lang/rust#114048) - [Add `internal_features` lint for internal unstable features.] (rust-lang/rust#108955) - [Mention style for new syntax in tracking issue template.] (rust-lang/rust#113586)

rustbot assigned m-ou-se May 1, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 1, 2023

nyurik changed the title ~~This is a simpler subset of the #111030~~ assert_eq! message format (take 2a) May 1, 2023

nyurik mentioned this pull request May 1, 2023

assert_eq! message format (take 2) #111030

Closed

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 1, 2023

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 2, 2023

rustbot removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 23, 2023

m-ou-se added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 5, 2023

m-ou-se added I-needs-decision Issue: In need of a decision. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 5, 2023

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 15, 2023

This comment has been minimized.

Sign in to view

nyurik force-pushed the simpler-issue-94005 branch from afbb8ca to 950e3d9 Compare August 15, 2023 20:54

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 16, 2023

bors merged commit b531630 into rust-lang:master Aug 16, 2023

nyurik deleted the simpler-issue-94005 branch August 16, 2023 00:51

epage mentioned this pull request Aug 19, 2023

Custom panic messages in snapbox assert-rs/snapbox#223

Open

apiraino removed the to-announce Announce this issue on triage meeting label Aug 24, 2023

ryoqun mentioned this pull request Aug 31, 2023

Upgrades Rust to 1.72.0 & nightly-2023-08-25 solana-labs/solana#32961

Merged

gendx added a commit to gendx/stv-rs that referenced this pull request Sep 29, 2023

Migrate tests to Rust nightly 1.73.

e185203

Upstream changes: - "Stabilize thread local cell methods." rust-lang/rust#114689 - "Cleaner assert_eq! & assert_ne! panic messages" rust-lang/rust#111071

coderabbitai Bot mentioned this pull request Apr 28, 2026

docs(kiro): align IDE/CLI setup guidance (#0000) EffortlessMetrics/perl-lsp#7070

Merged

Uh oh!

Conversation

nyurik commented May 1, 2023 • edited by m-ou-se Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Old messages

New messages

Uh oh!

rustbot commented May 1, 2023

Uh oh!

m-ou-se commented May 1, 2023

Uh oh!

This comment has been minimized.

bors commented May 1, 2023

Uh oh!

bors commented May 1, 2023

Uh oh!

This comment has been minimized.

rust-timer commented May 2, 2023

Overall result: no relevant changes - no action needed

Uh oh!

nyurik commented May 2, 2023

Uh oh!

nyurik commented May 9, 2023

Uh oh!

apiraino commented May 23, 2023

Uh oh!

nyurik commented May 24, 2023

Uh oh!

nyurik commented May 27, 2023

Uh oh!

m-ou-se commented May 27, 2023

Uh oh!

nyurik commented May 27, 2023

Uh oh!

m-ou-se commented Jun 5, 2023

Uh oh!

nyurik commented Aug 15, 2023

Uh oh!

albertlarsan68 commented Aug 15, 2023

Uh oh!

nyurik commented Aug 15, 2023

Uh oh!

bors commented Aug 15, 2023

Uh oh!

This comment has been minimized.

nyurik commented Aug 15, 2023

Uh oh!

nyurik commented Aug 15, 2023

Uh oh!

m-ou-se commented Aug 15, 2023

Uh oh!

bors commented Aug 15, 2023

Uh oh!

bors commented Aug 15, 2023

Uh oh!

bors commented Aug 16, 2023

Uh oh!

nyurik commented Aug 16, 2023

Uh oh!

rust-timer commented Aug 16, 2023

Overall result: ❌ regressions - no action needed

Uh oh!

m-ou-se commented Aug 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

nyurik commented May 1, 2023 •

edited by m-ou-se

Loading