-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Never type unsafe lint improvements #125282
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Never type unsafe lint improvements #125282
Conversation
(libs team decided not to add `absurd` to std)
r? @wesleywiser rustbot has assigned @wesleywiser. Use |
reason: FutureIncompatibilityReason::FutureReleaseSemanticsChange, | ||
reference: "issue #123748 <https://github.com/rust-lang/rust/issues/123748>", | ||
}; | ||
report_in_external_macro |
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.
didnt know about this :0
Do we already have consensus from T-lang that this is OK to bump to a FCW? I can't remember the policy around that. If you can find something, could you link it? |
This comment has been minimized.
This comment has been minimized.
I'm not aware of any policies with FCWs, ig we could nominate for T-lang? |
Yeah, let's do that. Also would like their input if they prefer this start out as an edition warning or an FCW. |
d913b16
to
434221b
Compare
probably intuitively simply because no one has needed it yet |
The plan that was FCPed (twice) by the lang team in #123508 is:
My read is that the plan implies that this lint (about fallback affecting a generic that is passed to an Put differently, it would not make sense to lint harder against #125289 than against #123939 when the latter is probably the more critical lint. The meeting minutes from 2024-04-10 also confirm that people wanted these lints to fire in all editions. |
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.
OK in that case I think this is good to go
@bors r+ rollup |
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#125034 (Weekly `cargo update`) - rust-lang#125093 (Add `fn into_raw_with_allocator` to Rc/Arc/Weak.) - rust-lang#125282 (Never type unsafe lint improvements) - rust-lang#125301 (fix suggestion in E0373 for !Unpin coroutines) - rust-lang#125302 (defrost `RUST_MIN_STACK=ice rustc hello.rs`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#125282 - WaffleLapkin:never-type-unsafe-improvements, r=compiler-errors Never type unsafe lint improvements - Move linting code to a separate method - Remove mentions of `core::convert::absurd` (rust-lang#124311 was rejected) - Make the lint into FCW The last thing is a bit weird though. On one hand it should be `EditionSemanticsChange(2024)`, but on the other hand it shouldn't, because we also plan to break it on all editions some time later. _Also_, it's weird that we don't have `FutureReleaseSemanticsChangeReportInDeps`, IMO "this might cause UB in a future release" is important enough to be reported in deps... IMO we ought to have three enums instead of [`FutureIncompatibilityReason`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint_defs/enum.FutureIncompatibilityReason.html#): ```rust enum IncompatibilityWhen { FutureRelease, Edition(Edition), } enum IncompatibilyWhat { Error, SemanticChange, } enum IncompatibilityReportInDeps { No, Yes, } ``` Tracking: - rust-lang#123748
core::convert::absurd
(Addcore::convert::absurd
#124311 was rejected)The last thing is a bit weird though. On one hand it should be
EditionSemanticsChange(2024)
, but on the other hand it shouldn't, because we also plan to break it on all editions some time later. Also, it's weird that we don't haveFutureReleaseSemanticsChangeReportInDeps
, IMO "this might cause UB in a future release" is important enough to be reported in deps...IMO we ought to have three enums instead of
FutureIncompatibilityReason
:Tracking:
!
fall back to!
#123748