-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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.
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
Improve handling of assertion failures with very long conditions #116548
Conversation
@bors r+ |
… r=matthewjasper Improve handling of assertion failures with very long conditions It's not perfectly clear what the best behaviour is here, but I think this is an improvement. r? `@matthewjasper` cc `@m-ou-se`
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#115882 (improve the suggestion of `generic_bound_failure`) - rust-lang#116537 (Fix suggestion span involving wrongly placed generic arg on variant) - rust-lang#116543 (In smir `find_crates` returns `Vec<Crate>` instead of `Option<Crate>`) - rust-lang#116548 (Improve handling of assertion failures with very long conditions) - rust-lang#116556 (Sync rustc_codegen_cranelift) r? `@ghost` `@rustbot` modify labels: rollup
… r=matthewjasper Improve handling of assertion failures with very long conditions It's not perfectly clear what the best behaviour is here, but I think this is an improvement. r? ``@matthewjasper`` cc ``@m-ou-se``
The test failure was here:
The problem is that in some cases Other assert tests in In other words, a |
IIRC, you can set a header like exec-env: RUST_BACKTRACE=0 or so? That'll disable the backtrace independent of runner environment. |
The `\n` in the output is a little surprising. The next commit will deal with it.
The assertion in `assert-long-condition.rs` used to be fail like this, all on one line: ``` thread 'main' panicked at 'assertion failed: 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 + 11 + 12 + 13 + 14 + 15 + 16 + 17 + 18\n + 19 + 20 + 21 + 22 + 23 + 24 + 25 == 0', tests/ui/macros/assert-long-condition.rs:7:5 ``` The `\n` and subsequent indent is because the condition is pretty-printed, and the pretty-printer inserts a newline. Printing the newline in this way is arguably reasonable given that the message appears within single quotes, which is very similar to a string literal. However, after the assertion printing improvements that were released in 1.73, the assertion now fails like this: ``` thread 'main' panicked at tests/ui/macros/assert-long-condition.rs:7:5: assertion failed: 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 + 11 + 12 + 13 + 14 + 15 + 16 + 17 + 18\n + 19 + 20 + 21 + 22 + 23 + 24 + 25 == 0 ``` Now that there are no single quotes around the pretty-printed condition, the `\n` is quite strange. This commit gets rid of the `\n`, by removing the `escape_debug` done on the pretty-printed message. This results in the following: ``` thread 'main' panicked at tests/ui/macros/assert-long-condition.rs:7:5: assertion failed: 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 + 11 + 12 + 13 + 14 + 15 + 16 + 17 + 18 + 19 + 20 + 21 + 22 + 23 + 24 + 25 == 0 ``` The overly-large indent is still strange, but that's a separate pretty-printing issue. This change helps with rust-lang#108341.
a6abc50
to
7528fdc
Compare
Great suggestion, thank you. I have updated with this change. @bors r=matthewjasper |
…=matthewjasper Improve handling of assertion failures with very long conditions It's not perfectly clear what the best behaviour is here, but I think this is an improvement. r? `@matthewjasper` cc `@m-ou-se`
Here's another example: fn very_long_function_name(x: u32) -> u32 { x }
fn main() {
assert!(
{
let x = 1;
let y = 2;
very_long_function_name(x) + very_long_function_name(y)
} ==
{
let x = 0;
let y = 1;
very_long_function_name(x) + very_long_function_name(y)
}
);
} Previously it would produce a single line of output on failure:
Now it produces something much better:
|
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
@bors retry |
@bors rollup |
…=matthewjasper Improve handling of assertion failures with very long conditions It's not perfectly clear what the best behaviour is here, but I think this is an improvement. r? `@matthewjasper` cc `@m-ou-se`
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
@bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (091bb74): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis 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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 625.981s -> 627.172s (0.19%) |
48: Pull upstream master 2023 10 12 r=tshepang a=Dajamante * rust-lang/rust#113487 * rust-lang/rust#116506 * rust-lang/rust#116448 * rust-lang/rust#116640 * rust-lang/rust#116627 * rust-lang/rust#116597 * rust-lang/rust#116436 * rust-lang/rust#116315 * rust-lang/rust#116219 * rust-lang/rust#113218 * rust-lang/rust#115937 * rust-lang/rust#116014 * rust-lang/rust#116623 * rust-lang/rust#112818 * rust-lang/rust#115948 * rust-lang/rust#116622 * rust-lang/rust#116621 * rust-lang/rust#116612 * rust-lang/rust#116611 * rust-lang/rust#116530 * rust-lang/rust#95967 * rust-lang/rust#116578 * rust-lang/rust#113915 * rust-lang/rust#116605 * rust-lang/rust#116574 * rust-lang/rust#116560 * rust-lang/rust#116559 * rust-lang/rust#116503 * rust-lang/rust#116444 * rust-lang/rust#116250 * rust-lang/rust#109422 * rust-lang/rust#116598 * rust-lang/rust#116596 * rust-lang/rust#116595 * rust-lang/rust#116589 * rust-lang/rust#116586 * rust-lang/rust#116551 * rust-lang/rust#116409 * rust-lang/rust#116548 * rust-lang/rust#116366 * rust-lang/rust#109882 * rust-lang/rust#116497 * rust-lang/rust#116532 * rust-lang/rust#116569 * rust-lang/rust#116561 * rust-lang/rust#116556 * rust-lang/rust#116549 * rust-lang/rust#116543 * rust-lang/rust#116537 * rust-lang/rust#115882 * rust-lang/rust#116142 * rust-lang/rust#115238 * rust-lang/rust#116533 * rust-lang/rust#116096 * rust-lang/rust#116468 * rust-lang/rust#116515 * rust-lang/rust#116454 * rust-lang/rust#116183 * rust-lang/rust#116514 * rust-lang/rust#116509 * rust-lang/rust#116487 * rust-lang/rust#116486 * rust-lang/rust#116450 * rust-lang/rust#114623 * rust-lang/rust#116416 * rust-lang/rust#116437 * rust-lang/rust#100806 * rust-lang/rust#116330 * rust-lang/rust#116310 * rust-lang/rust#115583 * rust-lang/rust#116457 * rust-lang/rust#116508 * rust-lang/rust#109214 * rust-lang/rust#116318 * rust-lang/rust#116501 * rust-lang/rust#116500 * rust-lang/rust#116458 * rust-lang/rust#116400 * rust-lang/rust#116277 * rust-lang/rust#114709 * rust-lang/rust#116492 * rust-lang/rust#116484 * rust-lang/rust#116481 * rust-lang/rust#116474 * rust-lang/rust#116466 * rust-lang/rust#116423 * rust-lang/rust#116297 * rust-lang/rust#114564 * rust-lang/rust#114811 * rust-lang/rust#116489 * rust-lang/rust#115304 Co-authored-by: Peter Hall <[email protected]> Co-authored-by: Emanuele Vannacci <[email protected]> Co-authored-by: Neven Villani <[email protected]> Co-authored-by: Alex Macleod <[email protected]> Co-authored-by: Tamir Duberstein <[email protected]> Co-authored-by: Eduardo Sánchez Muñoz <[email protected]> Co-authored-by: koka <[email protected]> Co-authored-by: bors <[email protected]> Co-authored-by: Philipp Krones <[email protected]> Co-authored-by: Camille GILLOT <[email protected]> Co-authored-by: Esteban Küber <[email protected]> Co-authored-by: Ralf Jung <[email protected]>
48: Pull upstream master 2023 10 12 r=tshepang a=Dajamante * rust-lang/rust#113487 * rust-lang/rust#116506 * rust-lang/rust#116448 * rust-lang/rust#116640 * rust-lang/rust#116627 * rust-lang/rust#116597 * rust-lang/rust#116436 * rust-lang/rust#116315 * rust-lang/rust#116219 * rust-lang/rust#113218 * rust-lang/rust#115937 * rust-lang/rust#116014 * rust-lang/rust#116623 * rust-lang/rust#112818 * rust-lang/rust#115948 * rust-lang/rust#116622 * rust-lang/rust#116621 * rust-lang/rust#116612 * rust-lang/rust#116611 * rust-lang/rust#116530 * rust-lang/rust#95967 * rust-lang/rust#116578 * rust-lang/rust#113915 * rust-lang/rust#116605 * rust-lang/rust#116574 * rust-lang/rust#116560 * rust-lang/rust#116559 * rust-lang/rust#116503 * rust-lang/rust#116444 * rust-lang/rust#116250 * rust-lang/rust#109422 * rust-lang/rust#116598 * rust-lang/rust#116596 * rust-lang/rust#116595 * rust-lang/rust#116589 * rust-lang/rust#116586 * rust-lang/rust#116551 * rust-lang/rust#116409 * rust-lang/rust#116548 * rust-lang/rust#116366 * rust-lang/rust#109882 * rust-lang/rust#116497 * rust-lang/rust#116532 * rust-lang/rust#116569 * rust-lang/rust#116561 * rust-lang/rust#116556 * rust-lang/rust#116549 * rust-lang/rust#116543 * rust-lang/rust#116537 * rust-lang/rust#115882 * rust-lang/rust#116142 * rust-lang/rust#115238 * rust-lang/rust#116533 * rust-lang/rust#116096 * rust-lang/rust#116468 * rust-lang/rust#116515 * rust-lang/rust#116454 * rust-lang/rust#116183 * rust-lang/rust#116514 * rust-lang/rust#116509 * rust-lang/rust#116487 * rust-lang/rust#116486 * rust-lang/rust#116450 * rust-lang/rust#114623 * rust-lang/rust#116416 * rust-lang/rust#116437 * rust-lang/rust#100806 * rust-lang/rust#116330 * rust-lang/rust#116310 * rust-lang/rust#115583 * rust-lang/rust#116457 * rust-lang/rust#116508 * rust-lang/rust#109214 * rust-lang/rust#116318 * rust-lang/rust#116501 * rust-lang/rust#116500 * rust-lang/rust#116458 * rust-lang/rust#116400 * rust-lang/rust#116277 * rust-lang/rust#114709 * rust-lang/rust#116492 * rust-lang/rust#116484 * rust-lang/rust#116481 * rust-lang/rust#116474 * rust-lang/rust#116466 * rust-lang/rust#116423 * rust-lang/rust#116297 * rust-lang/rust#114564 * rust-lang/rust#114811 * rust-lang/rust#116489 * rust-lang/rust#115304 Co-authored-by: Emanuele Vannacci <[email protected]> Co-authored-by: Neven Villani <[email protected]> Co-authored-by: Alex Macleod <[email protected]> Co-authored-by: Tamir Duberstein <[email protected]> Co-authored-by: Eduardo Sánchez Muñoz <[email protected]> Co-authored-by: koka <[email protected]> Co-authored-by: bors <[email protected]> Co-authored-by: Philipp Krones <[email protected]> Co-authored-by: Camille GILLOT <[email protected]> Co-authored-by: Esteban Küber <[email protected]> Co-authored-by: Ralf Jung <[email protected]> Co-authored-by: ShE3py <[email protected]>
It's not perfectly clear what the best behaviour is here, but I think this is an improvement.
r? @matthewjasper
cc @m-ou-se