-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fix manual_assert and match_wild_err_arm for #![no_std]
and Rust 2021
#7851
Conversation
r? @flip1995 (rust-highfive has picked a reviewer for you, use r? to override) |
tests/ui/if_then_panic_core.rs
Outdated
@@ -0,0 +1,56 @@ | |||
// run-rustfix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should duplicate tests for lints, but instead create tests like core_macros
or 2018_macros
, etc. We only need to test different variations of the macro, not different cases of lints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe just run the test twice in different editions? I am not sure how to do it though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that sounds better. I think you can do it like this:
// revisions: edition2015 edition2018
// [edition2015] edition:2015
// [edition2018] edition:2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that revisions didn't quite work... It expects me to have a "if_then_panic.edition2021.rs" because I have a "if_then_panic.edition2021.stderr".
The CI error confuses me. I didn't touch these files... |
this means don't use |
Oh I don't know about the formatting errors... |
See #7871 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a small comment. Otherwise this LGTM.
May conflict with #7810
tests/ui/if_then_panic.fixed
Outdated
@@ -1,3 +1,6 @@ | |||
// revisions: edition2018 edition2021 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file still necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately yes. If the file isn't there compiletest will complain that the fixed file cannot be compiled.
☔ The latest upstream changes (presumably #7810) made this pull request unmergeable. Please resolve the merge conflicts. |
#![no_std]
and Rust 2021#![no_std]
and Rust 2021
☔ The latest upstream changes (presumably #7866) made this pull request unmergeable. Please resolve the merge conflicts. |
@bors r+ Thanks! |
📌 Commit 14e0390 has been approved by |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Rust 2015
std::panic!
has a wrapping block whilecore::panic!
and Rust 2021std::panic!
does not. See rust-lang/rust#88919 for details.Note that the test won't pass until clippy changes in rust-lang/rust#88860 is synced.
changelog: Fix [
manual_assert
] and [match_wild_err_arm
] for#![no_std]
and Rust 2021.Fixes #7723