Conversation
|
r? @nnethercote rustbot has assigned @nnethercote. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
What's the most recent crater run we have for this? |
#157082, but I don't expect significant changes since, rust-random/rand_core#82 is three weeks ago and I'm not aware of any other fixes. |
|
Okay. Please update the PR description with a link to that crater run and a summary of the ecosystem impact: which crates are most-effected, for which of them have semver-compatible fixed been released. |
This comment has been minimized.
This comment has been minimized.
544bf74 to
25f78ee
Compare
This comment has been minimized.
This comment has been minimized.
25f78ee to
4129a31
Compare
This comment has been minimized.
This comment has been minimized.
4129a31 to
832b203
Compare
|
cc @rust-lang/clippy |
|
r=me purely from a mechanical compiler implementation POV, but it sounds like this needs buy-in from other parties. |
|
I have nominated this for the relevant teams. @mejrs if you could say a bit more on why you want to get rid of this, that'd be good. We should have solid reasons for breaking change with that many crater regressions. |
I've updated the description. |
|
We discussed this in today's lang meeting. We're in favor of shipping this as FCW, warn in deps, deny by default. @rfcbot fcp merge lang |
|
@tmandry has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
|
@rfcbot reviewed |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
832b203 to
d3c6cfc
Compare
d3c6cfc to
554cfe6
Compare
This comment has been minimized.
This comment has been minimized.
554cfe6 to
763466c
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
View all comments
Back in #95956 we changed how stability checking of paths works, by checking the stability of all segments rather than just the last item in it. This meant that several items in the (unstable)
std::intrinsicsmodule, which were stable and re-exported elsewhere, could not be imported on stable anymore. By doing so, this broke everyone who imported (one example)transmutethroughstd::intrinsics::transmuterather thanstd::mem::transmute.To fix this regression, it also introduced the
rustc_allowed_through_unstable_modulesattribute, which is a hack to allow stable imports through unstable paths.We have been emitting the
deprecatedlint for this since Rust 1.86 (in #136434).A recent crater run showed that many crates still depend on this accidental stabilization:
rand-coreversions, new fixed minor versions released 3 weeks ago (after the crater run) in old rand_core versions import from core::intrinsics module rust-random/rand_core#82 (comment)trim-in-place, which has a new major (but not minor) fixed version (trim-in-place-0.1.7 usescore::intrinsicsimport. magiclen/trim-in-place#6)wide, has a fixed minor version (errors were in wide-0.7.32, fix is in 0.7.33)Note however that the number of root regressions is very low (<100); almost all regressions are in (outdated) dependencies. The deprecation warnings have gotten people to fix their own code, but they cannot know about issues in their dependencies.
However, supporting this hack in the compiler significantly complicates the implementation of stability checking. Thus I'd really like to be able to get rid of this hack. That can't happen with so many regressions, but a step towards that is to start linting more aggressively on it. This PR turns the lint into an FCW.
Tracking issue: #163160
Zulip thread: #t-lang > fcwing on rustc_allowed_through_unstable_modules imports @ 💬
I don't know whose territory this is - adding new lints is lang's, but what library imports are valid is up to libs?