Expand core::panic!() to always include { .. }.
#80848
Closed
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
std::panic!() always expanded to a `{..}` block, but core::panic!() did
not. Because of this subtle difference, the issue-5500-1 test passed
for std::panic!(), but not for core::panic!().
This makes them behave the same, and extends that test to also check
core::panic!().
|
(rust-highfive has picked a reviewer for you, use r? to override) |
|
The braces were added to So I'd rather remove the braces from the |
|
@petrochenkov If I remove the braces, // check-pass
struct TrieMapIterator<'a> {
node: &'a usize
}
fn main() {
let a = 5;
let _iter = TrieMapIterator{node: &a};
_iter.node = &panic!()
}Is that a bug? |
|
At least the original issue #5500 was about invalid code producing an ICE rather than about valid code not compiling, so the test requiring the braces is accidental to the same degree as the commit that introduced the braces. |
|
It also makes I suppose |
This was referenced Jan 9, 2021
@petrochenkov Regardless of that, this would change the type-checking behavior of |
|
Are there cases in which adding the braces to panic breaks code? We should drop the braces in the 2021-edition panic though, since we are changing its behavior anyway. |
|
📌 Commit 9178bf2 has been approved by |
|
@bors rollup=never for easier bisection and perf |
|
⌛ Testing commit 9178bf2 with merge e8b203f744aef93d4f7a31907249ccc936cb4b75... |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
|
Looks like |
|
@bors r- |
|
⌛ Testing commit 9178bf2 with merge 9334e4398c5fd844c76883d1c50b516c5656cc4b... |
|
@bors r- retry |
|
☔ The latest upstream changes (presumably #82076) made this pull request unmergeable. Please resolve the merge conflicts. |
|
The issue #80846 was closed so closing the PR as well. |
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.
std::panic!()always expanded to a{..}block, butcore::panic!()did not. Because of this subtle difference, theissue-5500-1test passed forstd::panic!(), but not forcore::panic!().This makes them behave the same, and extends that test to also check core::panic!().
This fixes #80846.
This is part of the effort to make
std::panic!()andcore::panic!()identical, as tracked in #80162.