Skip to content

track the version and PR in which a nightly feature was removed #141619

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

Closed
jyn514 opened this issue May 26, 2025 · 5 comments · Fixed by #141642
Closed

track the version and PR in which a nightly feature was removed #141619

jyn514 opened this issue May 26, 2025 · 5 comments · Fixed by #141642
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented May 26, 2025

Code

#![feature(external_doc)]
#![doc(include("README.md"))]

Current output

error[E0557]: feature has been removed
 --> src/lib.rs:1:12
  |
1 | #![feature(external_doc)]
  |            ^^^^^^^^^^^^ feature has been removed
  |
  = note: use #[doc = include_str!("filename")] instead, which handles macro invocations

error: unknown `doc` attribute `include`
 --> src/lib.rs:2:8
  |
2 | #![doc(include("README.md"))]
  |        ^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[deny(invalid_doc_attributes)]` on by default

Desired output

error[E0557]: feature has been removed
 --> src/lib.rs:1:12
  |
1 | #![feature(external_doc)]
  |            ^^^^^^^^^^^^ feature has been removed
  |
  = note: removed in 1.54 (you are using 1.89-nightly); see <https://github.com/rust-lang/rust/pull/85457>
  = note: use #[doc = include_str!("filename")] instead, which handles macro invocations

Rationale and extra context

this makes it more clear what is going on, and also gives people enough breadcrumbs to find more information. right now it is very hard to find info about removed features without trawling through the code (it took me about 15 minutes just now, and i am both the person who removed this feature and the one who stabilized its replacement).

saying the version also makes it more clear that "removed" is specifically about versions, and tells the person compiling the code that using an older toolchain will likely work. this matters because the person seeing this error is likely not the one who wrote the code, and possibly knows very little about rust's stability policy.

Other cases

Rust Version

1.89.0-nightly

(2025-05-25 283db70ace62a0ae704a)

Anything else?

No response

@jyn514 jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 26, 2025
@jyn514
Copy link
Member Author

jyn514 commented May 27, 2025

oh neat, we already track version:

(removed, abi_amdgpu_kernel, "1.77.0", Some(51575), None),

that makes the version part of this quite simple, and we can add pr info later.

mentoring instructions: in

// If the enabled feature has been removed, issue an error.
if let Some(f) = REMOVED_LANG_FEATURES.iter().find(|f| name == f.feature.name) {
sess.dcx().emit_err(FeatureRemoved {
span: mi.span(),
reason: f.reason.map(|reason| FeatureRemovedReason { reason }),
});
continue;
}
, add a note about when the feature was removed.

@rustbot label E-help-wanted E-easy

@rustbot rustbot added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. labels May 27, 2025
@xizheyin
Copy link
Contributor

@rustbot claim

@xizheyin
Copy link
Contributor

For removed feature, the version may be not accurate, is this some kind of historical problem?

pub struct Feature {
pub name: Symbol,
/// For unstable features: the version the feature was added in.
/// For accepted features: the version the feature got stabilized in.
/// For removed features we are inconsistent; sometimes this is the
/// version it got added, sometimes the version it got removed.
pub since: &'static str,
issue: Option<NonZero<u32>>,
}

@jyn514
Copy link
Member Author

jyn514 commented May 27, 2025

hm. ideally we would not give wrong info in the diagnostic. are you willing to go through the git history for each feature one by one and see when it was removed? i think it will not be as bad as it sounds, a github blame view will get you 80% of the info you need because each commit links to the PR it's from (and the PR has a version number). it might also be possible to script this.

if you are willing, please note down the PR number as you do this, that will save someone from having to do the work again.

@xizheyin
Copy link
Contributor

No problem, I've just submitted #141642 to finish this for now, not sure if we want to merge this PR first before we go back to gradually fixing the diagnostic info.

fmease added a commit to fmease/rust that referenced this issue Jun 10, 2025
Note the version and PR of removed features when using it

Fixes rust-lang#141619

I added the diagnostic information. Since all the current version information is present, it prints the version information anyway, as shown in tests/ui. And PR will not print if it is None, we can gradually add the PR links.

Split into two commits for easier review.

r? compiler

cc `@jyn514` Since you're on vocation in the review list, I can't r? you.
@bors bors closed this as completed in 2aea4b2 Jun 10, 2025
rust-timer added a commit that referenced this issue Jun 10, 2025
Rollup merge of #141642 - xizheyin:issue-141619, r=BoxyUwU

Note the version and PR of removed features when using it

Fixes #141619

I added the diagnostic information. Since all the current version information is present, it prints the version information anyway, as shown in tests/ui. And PR will not print if it is None, we can gradually add the PR links.

Split into two commits for easier review.

r? compiler

cc ``@jyn514`` Since you're on vocation in the review list, I can't r? you.
tgross35 pushed a commit to tgross35/compiler-builtins that referenced this issue Jun 14, 2025
Note the version and PR of removed features when using it

Fixes rust-lang/rust#141619

I added the diagnostic information. Since all the current version information is present, it prints the version information anyway, as shown in tests/ui. And PR will not print if it is None, we can gradually add the PR links.

Split into two commits for easier review.

r? compiler

cc ``@jyn514`` Since you're on vocation in the review list, I can't r? you.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants