format panic message only once #110721
Merged
format panic message only once #110721
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
|
This has the unfortunate limitation of forcing an allocation in the panic path even when one isn't necessary (e.g. when using |
|
@bors r+ |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Apr 24, 2023
format panic message only once For `panic!` and friends, the `std` panic runtime will always set the `.payload()` of `PanicInfo` to the formatted string. The linked issues show that formatting the message twice can cause problems, so we simply print the already formatted message instead of formatting it again. We can't remove the preformatted payload, because it can be observed by custom panic hooks. fixes rust-lang#110717 fixes rust-itertools/itertools#694 cc `@Amanieu` who broke this in rust-lang#109507
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Apr 24, 2023
format panic message only once For `panic!` and friends, the `std` panic runtime will always set the `.payload()` of `PanicInfo` to the formatted string. The linked issues show that formatting the message twice can cause problems, so we simply print the already formatted message instead of formatting it again. We can't remove the preformatted payload, because it can be observed by custom panic hooks. fixes rust-lang#110717 fixes rust-itertools/itertools#694 cc ``@Amanieu`` who broke this in rust-lang#109507
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Apr 24, 2023
format panic message only once For `panic!` and friends, the `std` panic runtime will always set the `.payload()` of `PanicInfo` to the formatted string. The linked issues show that formatting the message twice can cause problems, so we simply print the already formatted message instead of formatting it again. We can't remove the preformatted payload, because it can be observed by custom panic hooks. fixes rust-lang#110717 fixes rust-itertools/itertools#694 cc ```@Amanieu``` who broke this in rust-lang#109507
|
☔ The latest upstream changes (presumably #110789) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
|
The panic changes have been reverted, but it's still good to have this test. @bors r+ |
bors
added
the
S-waiting-on-bors
|
⌛ Testing commit cd398a6 with merge c61a497df072444f1e66487f5d409d142728bdc0... |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
jyn514
added
S-waiting-on-author
|
Failure seems unrelated. @bors retry |
bors
added
S-waiting-on-bors
|
⌛ Testing commit cd398a6 with merge 1938114e1de4ff6bb282ed61c1005776b9645202... |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
looks spurious again: "failed to get successful HTTP response from https://crates.io/api/v1/crates/strsim/0.10.0/download, got 503" |
|
@bors retry |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 28, 2023
Rollup of 7 pull requests Successful merges: - rust-lang#109702 (configure --set support list as arguments) - rust-lang#110620 (Document `const {}` syntax for `std::thread_local`.) - rust-lang#110721 (format panic message only once) - rust-lang#110881 (refactor(docs): remove macro resolution fallback) - rust-lang#110893 (remove inline const deadcode in typeck) - rust-lang#110898 (Remove unused std::sys_common::thread_local_key::Key) - rust-lang#110909 (Skip `rustc` version detection on macOS) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Formatting the panic message multiple times can cause problems for some real-world crates, so here's a test to ensure that we don't do that.
This was regressed in #109507 and reverted in #110782.
fixes #110717
fixes rust-itertools/itertools#694