Skip to content

add suggestion for rustc_allowed_through_unstable_modules attribute - #162155

Open
mejrs wants to merge 1 commit into
rust-lang:mainfrom
mejrs:rustc_allowed_through_unstable_modules
Open

add suggestion for rustc_allowed_through_unstable_modules attribute#162155
mejrs wants to merge 1 commit into
rust-lang:mainfrom
mejrs:rustc_allowed_through_unstable_modules

Conversation

@mejrs

@mejrs mejrs commented Sep 1, 2026

Copy link
Copy Markdown
Member

No description provided.

@rustbot

rustbot commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 1, 2026
@rustbot

rustbot commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 75 candidates
  • Random selection from 20 candidates

Comment on lines +17 to +20
use core::intrinsics::{
transmute as _,
//~^ ERROR use of deprecated import through accidentally stabilized module `intrinsics`
};

@mejrs mejrs Sep 1, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rustfix doesn't like duplicate fixes (this impl would emit it twice for e.g. use std::intrinsics::{copy, copy_nonoverlapping};, but there are only 16 crates in Ralf's crater run that do this so I don't care

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "doesn't like" mean?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you run ci rustfix on e.g. use core::intrinsics::{transmute, copy}; then compiletest panics with

thread '[ui] tests\ui\stability-attribute\accidentally-stable-intrinsics.rs' panicked at
src\tools\compiletest\src\runtest\ui.rs:99:17:
failed to apply suggestions for "C:\\Users\\bruno\\Rust\\rust\\tests\\ui\\stability-attribute\\accidentally-stable-intrinsics.rs"
with rustfix: cannot replace slice of data that was already replaced

So I think it still works, I just can't write a test for it without complicating the implementation significantly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does cargo fix do with such replacements?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, per the docs: https://doc.rust-lang.org/nightly/nightly-rustc/rustfix/fn.apply_suggestions.html

If a replacement in a suggestion exactly matches a replacement of a previously applied solution, that entire suggestion will be skipped without generating an error. This is currently done to alleviate issues like #51211, although it may be removed if that’s fixed deeper in the compiler.

So that means it doesn't do anything? I don't think it's worth fixing - these imports are very rare and having to keep track of already emitted lints would be a bit annoying. It should be easier to do once #161349 lands though.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to avoid us emitting diagnostics that cause cargo fix to ICE or do something wrong. That would be worth fixing. If cargo fix just ignores some of the fixes, that's not great but less concerning.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I figured out what exactly the problem was. It is because if you have

use core::intrinsics::{transmute, copy}; then one lint will suggest replacing it with mem but the other with ptr.

In that case rustfix will just do nothing. In principle that could be fixed, but it is hard because ast lowering flattens the import structure, so we don't really know multiple imports came from a single tree. #161349 possibly might make that easier. But given that there are no such cases in the crater run (I only see import tree combinations of copy/copy_nonoverlapping/write_bytes) I don't think it's worth complicating the implementation or waiting for that pr.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both suggestions are wrong though. I don't mind duplicate suggestions that much, but incorrect suggestions are a bug. Is that a good way to just not have a suggestion in this case?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. I guess we could, rather than emitting the lint immediately, collect all lints and at the end of the pass try to guess by looking at spans which imports came from the same use tree and not emit those suggestions. There are probably corner cases with that approach too.

So that's a decent implementation effort/complexity for something that ~never happens (no such cases in the crater run). And then Oli gets to rip it all out when he gets to implementing the unflattening of hir use trees.

I will not be wasting my time implementing that.

@rust-log-analyzer

This comment has been minimized.

@mejrs
mejrs force-pushed the rustc_allowed_through_unstable_modules branch from 260bc7c to 11abd8d Compare September 1, 2026 21:53
@rustbot rustbot added the T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. label Sep 1, 2026
@rust-log-analyzer

This comment has been minimized.

@mejrs
mejrs force-pushed the rustc_allowed_through_unstable_modules branch from 11abd8d to 4badc23 Compare September 2, 2026 09:01
@mejrs
mejrs force-pushed the rustc_allowed_through_unstable_modules branch from 4badc23 to 8f316d4 Compare September 3, 2026 10:42
@davidtwco

Copy link
Copy Markdown
Member

@bors r+ rollup

@rust-bors

rust-bors Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 8f316d4 has been approved by davidtwco

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2026
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 4, 2026
…le_modules, r=davidtwco

add suggestion for `rustc_allowed_through_unstable_modules` attribute
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 4, 2026
…le_modules, r=davidtwco

add suggestion for `rustc_allowed_through_unstable_modules` attribute
rust-bors Bot pushed a commit that referenced this pull request Sep 4, 2026
Rollup of 25 pull requests

Successful merges:

 - #159074 ([PAC] FnAbi, llvm.ptrauth.resign and Session API change (2/8))
 - #159792 (A more readable debug map for IndexMaps)
 - #161895 (std::sys::pal::sgx: fix mismatched alloc/free alignment)
 - #161900 (bootstrap: Include feature-gated items in bootstrap tool docs)
 - #161940 (Promote `wasm32-wasip3` to a tier 2 target)
 - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`)
 - #162179 (type system const items via direct rhs)
 - #162277 (Introduce `rustc_middle::middel::resolve`)
 - #162285 (box: fixup map/try_map deallocate calls)
 - #162286 (string: don't unwind prematurely)
 - #162289 (alloc: a bunch of safety comments)
 - #162292 (Update `askama` version to `0.16.1`)
 - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`)
 - #160906 (Suggest usize instead of placeholder type for array length constants)
 - #160936 (traits: Represent live alias arguments as bitsets)
 - #161400 (Improve diagnostics for references to closures)
 - #161656 (Suggest mutable references for FnMut closure arguments)
 - #161711 (Add more splat fn type tests)
 - #161786 (Make `tcx.def_id_partial_cmp` public)
 - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers)
 - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute)
 - #162212 (Implement `Rng` for `Box`)
 - #162246 (Fix incorrect meta span)
 - #162266 (std: fix typo)
 - #162291 (Add regression test from 1.98.1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants