Skip to content

Commit 456d23e

Browse files
committed
Add a lint for writing #[feature] for stable features, warn by default.
The 'stable_features' lint helps people progress from unstable to stable Rust by telling them when they no longer need a `feature` attribute because upstream Rust has declared it stable. This compares to the existing 'unstable_features', which is used to implement feature staging, and triggers on *any* use of `#[feature]`.
1 parent ba2f13e commit 456d23e

File tree

6 files changed

+77
-23
lines changed

6 files changed

+77
-23
lines changed

src/librustc/lint/builtin.rs

+7
Original file line numberDiff line numberDiff line change
@@ -2018,6 +2018,12 @@ declare_lint! {
20182018
"unused or unknown features found in crate-level #[feature] directives"
20192019
}
20202020

2021+
declare_lint! {
2022+
pub STABLE_FEATURES,
2023+
Warn,
2024+
"stable features found in #[feature] directive"
2025+
}
2026+
20212027
declare_lint! {
20222028
pub UNKNOWN_CRATE_TYPES,
20232029
Deny,
@@ -2060,6 +2066,7 @@ impl LintPass for HardwiredLints {
20602066
UNREACHABLE_CODE,
20612067
WARNINGS,
20622068
UNUSED_FEATURES,
2069+
STABLE_FEATURES,
20632070
UNKNOWN_CRATE_TYPES,
20642071
VARIANT_SIZE_DIFFERENCES,
20652072
FAT_PTR_TRANSMUTES

src/librustc/middle/stability.rs

+38-15
Original file line numberDiff line numberDiff line change
@@ -201,16 +201,17 @@ impl Index {
201201
/// Cross-references the feature names of unstable APIs with enabled
202202
/// features and possibly prints errors. Returns a list of all
203203
/// features used.
204-
pub fn check_unstable_api_usage(tcx: &ty::ctxt) -> FnvHashSet<InternedString> {
205-
let ref active_lib_features = tcx.sess.features.borrow().lib_features;
204+
pub fn check_unstable_api_usage(tcx: &ty::ctxt)
205+
-> FnvHashMap<InternedString, attr::StabilityLevel> {
206+
let ref active_lib_features = tcx.sess.features.borrow().declared_lib_features;
206207

207208
// Put the active features into a map for quick lookup
208209
let active_features = active_lib_features.iter().map(|&(ref s, _)| s.clone()).collect();
209210

210211
let mut checker = Checker {
211212
tcx: tcx,
212213
active_features: active_features,
213-
used_features: FnvHashSet()
214+
used_features: FnvHashMap()
214215
};
215216

216217
let krate = tcx.map.krate();
@@ -223,7 +224,7 @@ pub fn check_unstable_api_usage(tcx: &ty::ctxt) -> FnvHashSet<InternedString> {
223224
struct Checker<'a, 'tcx: 'a> {
224225
tcx: &'a ty::ctxt<'tcx>,
225226
active_features: FnvHashSet<InternedString>,
226-
used_features: FnvHashSet<InternedString>
227+
used_features: FnvHashMap<InternedString, attr::StabilityLevel>
227228
}
228229

229230
impl<'a, 'tcx> Checker<'a, 'tcx> {
@@ -234,7 +235,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
234235

235236
match *stab {
236237
Some(Stability { level: attr::Unstable, ref feature, ref reason, .. }) => {
237-
self.used_features.insert(feature.clone());
238+
self.used_features.insert(feature.clone(), attr::Unstable);
238239

239240
if !self.active_features.contains(feature) {
240241
let msg = match *reason {
@@ -247,7 +248,9 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
247248
feature.get(), span, &msg[]);
248249
}
249250
}
250-
Some(..) => {
251+
Some(Stability { level, ref feature, .. }) => {
252+
self.used_features.insert(feature.clone(), level);
253+
251254
// Stable APIs are always ok to call and deprecated APIs are
252255
// handled by a lint.
253256
}
@@ -433,17 +436,37 @@ pub fn lookup(tcx: &ty::ctxt, id: DefId) -> Option<Stability> {
433436
/// Given the list of enabled features that were not language features (i.e. that
434437
/// were expected to be library features), and the list of features used from
435438
/// libraries, identify activated features that don't exist and error about them.
436-
pub fn check_unused_features(sess: &Session,
437-
used_lib_features: &FnvHashSet<InternedString>) {
438-
let ref lib_features = sess.features.borrow().lib_features;
439-
let mut active_lib_features: FnvHashMap<InternedString, Span>
440-
= lib_features.clone().into_iter().collect();
441-
442-
for used_feature in used_lib_features {
443-
active_lib_features.remove(used_feature);
439+
pub fn check_unused_or_stable_features(sess: &Session,
440+
lib_features_used: &FnvHashMap<InternedString,
441+
attr::StabilityLevel>) {
442+
let ref declared_lib_features = sess.features.borrow().declared_lib_features;
443+
let mut remaining_lib_features: FnvHashMap<InternedString, Span>
444+
= declared_lib_features.clone().into_iter().collect();
445+
446+
let stable_msg = "this feature is stable. attribute no longer needed";
447+
448+
for &span in sess.features.borrow().declared_stable_lang_features.iter() {
449+
sess.add_lint(lint::builtin::STABLE_FEATURES,
450+
ast::CRATE_NODE_ID,
451+
span,
452+
stable_msg.to_string());
453+
}
454+
455+
for (used_lib_feature, level) in lib_features_used.iter() {
456+
match remaining_lib_features.remove(used_lib_feature) {
457+
Some(span) => {
458+
if *level == attr::Stable {
459+
sess.add_lint(lint::builtin::STABLE_FEATURES,
460+
ast::CRATE_NODE_ID,
461+
span,
462+
stable_msg.to_string());
463+
}
464+
}
465+
None => ( /* used but undeclared, handled during the previous ast visit */ )
466+
}
444467
}
445468

446-
for (_, &span) in &active_lib_features {
469+
for (_, &span) in remaining_lib_features.iter() {
447470
sess.add_lint(lint::builtin::UNUSED_FEATURES,
448471
ast::CRATE_NODE_ID,
449472
span,

src/librustc_driver/driver.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -668,8 +668,8 @@ pub fn phase_3_run_analysis_passes<'tcx>(sess: Session,
668668
time(time_passes, "stability checking", (), |_|
669669
stability::check_unstable_api_usage(&ty_cx));
670670

671-
time(time_passes, "unused feature checking", (), |_|
672-
stability::check_unused_features(
671+
time(time_passes, "unused lib feature checking", (), |_|
672+
stability::check_unused_or_stable_features(
673673
&ty_cx.sess, lib_features_used));
674674

675675
time(time_passes, "lint checking", (), |_|

src/libsyntax/feature_gate.rs

+10-5
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,10 @@ pub struct Features {
149149
pub old_orphan_check: bool,
150150
pub simd_ffi: bool,
151151
pub unmarked_api: bool,
152-
pub lib_features: Vec<(InternedString, Span)>
152+
/// spans of #![feature] attrs for stable language features. for error reporting
153+
pub declared_stable_lang_features: Vec<Span>,
154+
/// #![feature] attrs for non-language (library) features
155+
pub declared_lib_features: Vec<(InternedString, Span)>
153156
}
154157

155158
impl Features {
@@ -162,7 +165,8 @@ impl Features {
162165
old_orphan_check: false,
163166
simd_ffi: false,
164167
unmarked_api: false,
165-
lib_features: Vec::new()
168+
declared_stable_lang_features: Vec::new(),
169+
declared_lib_features: Vec::new()
166170
}
167171
}
168172
}
@@ -511,6 +515,7 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::C
511515
cm: cm,
512516
};
513517

518+
let mut accepted_features = Vec::new();
514519
let mut unknown_features = Vec::new();
515520

516521
for attr in &krate.attrs {
@@ -550,8 +555,7 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::C
550555
span_handler.span_err(mi.span, "feature has been removed");
551556
}
552557
Some(&(_, _, Accepted)) => {
553-
span_handler.span_warn(mi.span, "feature has been added to Rust, \
554-
directive not necessary");
558+
accepted_features.push(mi.span);
555559
}
556560
None => {
557561
unknown_features.push((name, mi.span));
@@ -572,7 +576,8 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::C
572576
old_orphan_check: cx.has_feature("old_orphan_check"),
573577
simd_ffi: cx.has_feature("simd_ffi"),
574578
unmarked_api: cx.has_feature("unmarked_api"),
575-
lib_features: unknown_features
579+
declared_stable_lang_features: accepted_features,
580+
declared_lib_features: unknown_features
576581
}
577582
}
578583

src/test/compile-fail/gated-bad-feature.rs

-1
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,3 @@
2020
#![feature = "foo"] //~ ERROR: malformed feature
2121

2222
#![feature(test_removed_feature)] //~ ERROR: feature has been removed
23-
#![feature(test_accepted_feature)] //~ WARNING: feature has been added
+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Testing that the stable_features lint catches use of stable
12+
// language and lib features.
13+
14+
#![deny(stable_features)]
15+
#![feature(test_accepted_feature)] //~ ERROR this feature is stable
16+
#![feature(rust1)] //~ ERROR this feature is stable
17+
18+
fn main() {
19+
let _foo: Vec<()> = Vec::new();
20+
}

0 commit comments

Comments
 (0)