Skip to content

Add a lint for writing #[feature] for stable features, warn by default... #21958

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

Merged
merged 1 commit into from
Feb 7, 2015
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
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2018,6 +2018,12 @@ declare_lint! {
"unused or unknown features found in crate-level #[feature] directives"
}

declare_lint! {
pub STABLE_FEATURES,
Warn,
"stable features found in #[feature] directive"
}

declare_lint! {
pub UNKNOWN_CRATE_TYPES,
Deny,
Expand Down Expand Up @@ -2060,6 +2066,7 @@ impl LintPass for HardwiredLints {
UNREACHABLE_CODE,
WARNINGS,
UNUSED_FEATURES,
STABLE_FEATURES,
UNKNOWN_CRATE_TYPES,
VARIANT_SIZE_DIFFERENCES,
FAT_PTR_TRANSMUTES
Expand Down
53 changes: 38 additions & 15 deletions src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,16 +201,17 @@ impl Index {
/// Cross-references the feature names of unstable APIs with enabled
/// features and possibly prints errors. Returns a list of all
/// features used.
pub fn check_unstable_api_usage(tcx: &ty::ctxt) -> FnvHashSet<InternedString> {
let ref active_lib_features = tcx.sess.features.borrow().lib_features;
pub fn check_unstable_api_usage(tcx: &ty::ctxt)
-> FnvHashMap<InternedString, attr::StabilityLevel> {
let ref active_lib_features = tcx.sess.features.borrow().declared_lib_features;

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

let mut checker = Checker {
tcx: tcx,
active_features: active_features,
used_features: FnvHashSet()
used_features: FnvHashMap()
};

let krate = tcx.map.krate();
Expand All @@ -223,7 +224,7 @@ pub fn check_unstable_api_usage(tcx: &ty::ctxt) -> FnvHashSet<InternedString> {
struct Checker<'a, 'tcx: 'a> {
tcx: &'a ty::ctxt<'tcx>,
active_features: FnvHashSet<InternedString>,
used_features: FnvHashSet<InternedString>
used_features: FnvHashMap<InternedString, attr::StabilityLevel>
}

impl<'a, 'tcx> Checker<'a, 'tcx> {
Expand All @@ -234,7 +235,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {

match *stab {
Some(Stability { level: attr::Unstable, ref feature, ref reason, .. }) => {
self.used_features.insert(feature.clone());
self.used_features.insert(feature.clone(), attr::Unstable);

if !self.active_features.contains(feature) {
let msg = match *reason {
Expand All @@ -247,7 +248,9 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
feature.get(), span, &msg[]);
}
}
Some(..) => {
Some(Stability { level, ref feature, .. }) => {
self.used_features.insert(feature.clone(), level);

// Stable APIs are always ok to call and deprecated APIs are
// handled by a lint.
}
Expand Down Expand Up @@ -433,17 +436,37 @@ pub fn lookup(tcx: &ty::ctxt, id: DefId) -> Option<Stability> {
/// Given the list of enabled features that were not language features (i.e. that
/// were expected to be library features), and the list of features used from
/// libraries, identify activated features that don't exist and error about them.
pub fn check_unused_features(sess: &Session,
used_lib_features: &FnvHashSet<InternedString>) {
let ref lib_features = sess.features.borrow().lib_features;
let mut active_lib_features: FnvHashMap<InternedString, Span>
= lib_features.clone().into_iter().collect();

for used_feature in used_lib_features {
active_lib_features.remove(used_feature);
pub fn check_unused_or_stable_features(sess: &Session,
lib_features_used: &FnvHashMap<InternedString,
attr::StabilityLevel>) {
let ref declared_lib_features = sess.features.borrow().declared_lib_features;
let mut remaining_lib_features: FnvHashMap<InternedString, Span>
= declared_lib_features.clone().into_iter().collect();

let stable_msg = "this feature is stable. attribute no longer needed";

for &span in sess.features.borrow().declared_stable_lang_features.iter() {
sess.add_lint(lint::builtin::STABLE_FEATURES,
ast::CRATE_NODE_ID,
span,
stable_msg.to_string());
}

for (used_lib_feature, level) in lib_features_used.iter() {
match remaining_lib_features.remove(used_lib_feature) {
Some(span) => {
if *level == attr::Stable {
sess.add_lint(lint::builtin::STABLE_FEATURES,
ast::CRATE_NODE_ID,
span,
stable_msg.to_string());
}
}
None => ( /* used but undeclared, handled during the previous ast visit */ )
}
}

for (_, &span) in &active_lib_features {
for (_, &span) in remaining_lib_features.iter() {
sess.add_lint(lint::builtin::UNUSED_FEATURES,
ast::CRATE_NODE_ID,
span,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,8 +668,8 @@ pub fn phase_3_run_analysis_passes<'tcx>(sess: Session,
time(time_passes, "stability checking", (), |_|
stability::check_unstable_api_usage(&ty_cx));

time(time_passes, "unused feature checking", (), |_|
stability::check_unused_features(
time(time_passes, "unused lib feature checking", (), |_|
stability::check_unused_or_stable_features(
&ty_cx.sess, lib_features_used));

time(time_passes, "lint checking", (), |_|
Expand Down
15 changes: 10 additions & 5 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,10 @@ pub struct Features {
pub old_orphan_check: bool,
pub simd_ffi: bool,
pub unmarked_api: bool,
pub lib_features: Vec<(InternedString, Span)>
/// spans of #![feature] attrs for stable language features. for error reporting
pub declared_stable_lang_features: Vec<Span>,
/// #![feature] attrs for non-language (library) features
pub declared_lib_features: Vec<(InternedString, Span)>
}

impl Features {
Expand All @@ -162,7 +165,8 @@ impl Features {
old_orphan_check: false,
simd_ffi: false,
unmarked_api: false,
lib_features: Vec::new()
declared_stable_lang_features: Vec::new(),
declared_lib_features: Vec::new()
}
}
}
Expand Down Expand Up @@ -511,6 +515,7 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::C
cm: cm,
};

let mut accepted_features = Vec::new();
let mut unknown_features = Vec::new();

for attr in &krate.attrs {
Expand Down Expand Up @@ -550,8 +555,7 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::C
span_handler.span_err(mi.span, "feature has been removed");
}
Some(&(_, _, Accepted)) => {
span_handler.span_warn(mi.span, "feature has been added to Rust, \
directive not necessary");
accepted_features.push(mi.span);
}
None => {
unknown_features.push((name, mi.span));
Expand All @@ -572,7 +576,8 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::C
old_orphan_check: cx.has_feature("old_orphan_check"),
simd_ffi: cx.has_feature("simd_ffi"),
unmarked_api: cx.has_feature("unmarked_api"),
lib_features: unknown_features
declared_stable_lang_features: accepted_features,
declared_lib_features: unknown_features
}
}

Expand Down
1 change: 0 additions & 1 deletion src/test/compile-fail/gated-bad-feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,3 @@
#![feature = "foo"] //~ ERROR: malformed feature

#![feature(test_removed_feature)] //~ ERROR: feature has been removed
#![feature(test_accepted_feature)] //~ WARNING: feature has been added
20 changes: 20 additions & 0 deletions src/test/compile-fail/stable-features.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Testing that the stable_features lint catches use of stable
// language and lib features.

#![deny(stable_features)]
#![feature(test_accepted_feature)] //~ ERROR this feature is stable
#![feature(rust1)] //~ ERROR this feature is stable

fn main() {
let _foo: Vec<()> = Vec::new();
}