Refactor how lints define future incompatibility - #163555
WaffleLapkin wants to merge 4 commits into
Conversation
|
The rustc-dev-guide subtree was changed. If your future PRs only touch the subtree, consider submitting them directly to rust-lang/rustc-dev-guide, which is where the document is primarily maintained (and has faster CI). |
rustbot
added
A-rustc-dev-guide
|
|
| const fn edition_from_u16(x: u16) -> Edition { | ||
| match x { | ||
| 2015 => Edition::Edition2015, | ||
| 2018 => Edition::Edition2018, | ||
| 2021 => Edition::Edition2021, | ||
| 2024 => Edition::Edition2024, | ||
| _ => panic!("invalid edition number"), | ||
| } | ||
| } |
There was a problem hiding this comment.
This feels like a hack tbh. Maybe we should just accept Edition and use use Edition::* on the callsite?
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
This seems fine, but it is touching the edition logic that I don't know much about.
r? @jdonszelmann maybe?
| /// After a lint has been in this state for a while, consider setting this to true, so it | ||
| /// warns for everyone. It is a good signal that it is ready if you can determine that all |
There was a problem hiding this comment.
| /// After a lint has been in this state for a while, consider setting this to true, so it | |
| /// warns for everyone. It is a good signal that it is ready if you can determine that all | |
| /// After a lint has been in this state for a while, consider setting this to true, so it | |
| /// warns for everyone. Typically, the default lint level is set to `Deny` at that point. | |
| /// It is a good signal that it is ready if you can determine that all |
| /// [`EditionSemanticsChange`]: FutureIncompatibilityReason::EditionSemanticsChange | ||
| /// [`FutureReleaseSemanticsChange`]: FutureIncompatibilityReason::FutureReleaseSemanticsChange | ||
| EditionAndFutureReleaseSemanticsChange(EditionFcw), | ||
| EditionAndFutureReleaseSemanticsChange(EditionFcw, ReleaseFcw), |
There was a problem hiding this comment.
Why did this change? Is this variant even used?
There was a problem hiding this comment.
This changed because this represents both an edition and a future release change (i.e. what we did with some never type things). For this kind of change it is reasonable to report it in dependencies, and since that is now stored in ReleaseFcw, it also needs to store that. (IMO it should have already stored it, but oh well).
It is not currently used, no.
There was a problem hiding this comment.
But this means we're storing the issue number twice, doesn't it?
| ..$crate::Lint::default_fields_for_macro() | ||
| $vis static $NAME: &$crate::Lint = { | ||
| #[allow(unused_imports)] | ||
| use $crate::{future_release_error, future_release_semantics_change, edition_error, edition_semantics_change}; |
There was a problem hiding this comment.
Regarding #163555 (comment), the edition enum variants could be imported here?
|
|
This branch has not been deployed
5 participants
The main changes are:
report_in_depsrequired, as suggested by Add FCW for invalid C variadic arguments #162478 (comment)r? @RalfJung
cc @jdonszelmann