Skip to content
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

Suggest removing #![feature] for library features that have been stabilized #89012

Merged
merged 1 commit into from
Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,10 +702,16 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session) {
}

fn maybe_stage_features(sess: &Session, krate: &ast::Crate) {
// checks if `#![feature]` has been used to enable any lang feature
// does not check the same for lib features unless there's at least one
// declared lang feature
use rustc_errors::Applicability;

if !sess.opts.unstable_features.is_nightly_build() {
let lang_features = &sess.features_untracked().declared_lang_features;
if lang_features.len() == 0 {
return;
Copy link
Member

Choose a reason for hiding this comment

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

What is this check doing? It should never be hit since there will always be some features built-in, but it also doesn't seem to have anything to do with the rest of the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The lang_features vector contains the declared_lang_features in the session. So only the ones which have been enabled show up here.
I've added this check to skip emitting an error if only lib_features have been enabled and no lang_features

Copy link
Member

Choose a reason for hiding this comment

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

Ok. That still seems like the wrong check, though - if there's one language and one library feature, it should still suggest removing the stable library feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How will that be possible since the suggestion can be generated only after TyCtxt is generated? And we will be throwing an error at the parsing stage if a lang feature has been enabled, the compiler will abort and won't go to the next stage

Copy link
Member

Choose a reason for hiding this comment

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

And we will be throwing an error at the parsing stage if a lang feature has been enabled, the compiler will abort and won't go to the next stage

Oh hmm, good point. Ok, this seems fine then - if the language feature wasn't stable removing the library feature wouldn't have helped anyway.

}
for attr in krate.attrs.iter().filter(|attr| attr.has_name(sym::feature)) {
let mut err = struct_span_err!(
sess.parse_sess.span_diagnostic,
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,16 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
let declared_lib_features = &tcx.features().declared_lib_features;
let mut remaining_lib_features = FxHashMap::default();
for (feature, span) in declared_lib_features {
if !tcx.sess.opts.unstable_features.is_nightly_build() {
struct_span_err!(
tcx.sess,
*span,
E0554,
"`#![feature]` may not be used on the {} release channel",
env!("CFG_RELEASE_CHANNEL")
)
.emit();
}
if remaining_lib_features.contains_key(&feature) {
// Warn if the user enables a lib feature multiple times.
duplicate_feature_err(tcx.sess, *span, *feature);
Expand Down