From 5242dce01db3fc42d5ec91eb166cce83194149cb Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 23 Jul 2018 01:20:33 +0100 Subject: [PATCH] Add lint for unknown feature attributes --- src/librustc/dep_graph/dep_node.rs | 2 + src/librustc/diagnostics.rs | 1 + src/librustc/ich/impls_hir.rs | 5 + src/librustc/lib.rs | 1 + src/librustc/lint/builtin.rs | 9 +- src/librustc/middle/lib_features.rs | 153 +++++++++++++++++++++++++++ src/librustc/middle/stability.rs | 57 +++++----- src/librustc/ty/context.rs | 9 ++ src/librustc/ty/query/config.rs | 12 +++ src/librustc/ty/query/mod.rs | 8 ++ src/librustc/ty/query/plumbing.rs | 2 + src/librustc_lint/lib.rs | 2 +- src/librustc_metadata/cstore_impl.rs | 1 + src/librustc_metadata/decoder.rs | 8 ++ src/librustc_metadata/encoder.rs | 13 +++ src/librustc_metadata/schema.rs | 1 + src/libsyntax/diagnostic_list.rs | 12 +++ 17 files changed, 268 insertions(+), 28 deletions(-) create mode 100644 src/librustc/middle/lib_features.rs diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 14a818ddafb..f099325dfc1 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -615,6 +615,8 @@ pub fn fingerprint_needed_for_crate_hash(self) -> bool { [input] CrateName(CrateNum), [] ItemChildren(DefId), [] ExternModStmtCnum(DefId), + [input] GetLibFeatures, + [] DefinedLibFeatures(CrateNum), [input] GetLangItems, [] DefinedLangItems(CrateNum), [] MissingLangItems(CrateNum), diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index 5ace8397d9f..3b3be757882 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -2138,4 +2138,5 @@ trait Foo { } E0708, // `async` non-`move` closures with arguments are not currently supported E0709, // multiple different lifetimes used in arguments of `async fn` E0710, // an unknown tool name found in scoped lint + E0711, // a feature has been declared with conflicting stability attributes } diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index cac0d182d3a..fe87053681a 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -1072,6 +1072,11 @@ fn to_stable_hash_key(&self, hcx: &StableHashingContext<'a>) -> DefPathHash { span }); +impl_stable_hash_for!(struct ::middle::lib_features::LibFeatures { + stable, + unstable +}); + impl<'a> HashStable> for ::middle::lang_items::LangItem { fn hash_stable(&self, _: &mut StableHashingContext<'a>, diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index bac511aac1f..8ff5d33c91d 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -141,6 +141,7 @@ pub mod middle { pub mod exported_symbols; pub mod free_region; pub mod intrinsicck; + pub mod lib_features; pub mod lang_items; pub mod liveness; pub mod mem_categorization; diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 47f7c759d94..6536ab6ea73 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -102,7 +102,13 @@ declare_lint! { pub UNUSED_FEATURES, Warn, - "unused or unknown features found in crate-level #[feature] directives" + "unused features found in crate-level #[feature] directives" +} + +declare_lint! { + pub UNKNOWN_FEATURES, + Deny, + "unknown features found in crate-level #[feature] directives" } declare_lint! { @@ -362,6 +368,7 @@ fn get_lints(&self) -> LintArray { UNUSED_MACROS, WARNINGS, UNUSED_FEATURES, + UNKNOWN_FEATURES, STABLE_FEATURES, UNKNOWN_CRATE_TYPES, TRIVIAL_CASTS, diff --git a/src/librustc/middle/lib_features.rs b/src/librustc/middle/lib_features.rs new file mode 100644 index 00000000000..27bd69f04fd --- /dev/null +++ b/src/librustc/middle/lib_features.rs @@ -0,0 +1,153 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Detecting lib features (i.e. features that are not lang features). +// +// These are declared using stability attributes (e.g. `#[stable(..)]` +// and `#[unstable(..)]`), but are not declared in one single location +// (unlike lang features), which means we need to collect them instead. + +use ty::TyCtxt; +use syntax::symbol::Symbol; +use syntax::ast::{Attribute, MetaItem, MetaItemKind}; +use syntax_pos::{Span, DUMMY_SP}; +use hir; +use hir::itemlikevisit::ItemLikeVisitor; +use rustc_data_structures::fx::{FxHashSet, FxHashMap}; +use errors::DiagnosticId; + +pub struct LibFeatures { + // A map from feature to stabilisation version. + pub stable: FxHashMap, + pub unstable: FxHashSet, +} + +impl LibFeatures { + fn new() -> LibFeatures { + LibFeatures { + stable: FxHashMap(), + unstable: FxHashSet(), + } + } + + pub fn iter(&self) -> Vec<(Symbol, Option)> { + self.stable.iter().map(|(f, s)| (*f, Some(*s))) + .chain(self.unstable.iter().map(|f| (*f, None))) + .collect() + } +} + +pub struct LibFeatureCollector<'a, 'tcx: 'a> { + tcx: TyCtxt<'a, 'tcx, 'tcx>, + lib_features: LibFeatures, +} + +impl<'a, 'tcx> LibFeatureCollector<'a, 'tcx> { + fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> LibFeatureCollector<'a, 'tcx> { + LibFeatureCollector { + tcx, + lib_features: LibFeatures::new(), + } + } + + fn extract(&self, attrs: &[Attribute]) -> Vec<(Symbol, Option, Span)> { + let stab_attrs = vec!["stable", "unstable", "rustc_const_unstable"]; + let mut features = vec![]; + + for attr in attrs { + // FIXME(varkor): the stability attribute might be behind a `#[cfg]` attribute. + + // Find a stability attribute (i.e. `#[stable(..)]`, `#[unstable(..)]`, + // `#[rustc_const_unstable(..)]`). + if stab_attrs.iter().any(|stab_attr| attr.check_name(stab_attr)) { + let meta_item = attr.meta(); + if let Some(MetaItem { node: MetaItemKind::List(ref metas), .. }) = meta_item { + let mut feature = None; + let mut since = None; + for meta in metas { + if let Some(mi) = meta.meta_item() { + // Find the `feature = ".."` meta-item. + match (&*mi.name().as_str(), mi.value_str()) { + ("feature", val) => feature = val, + ("since", val) => since = val, + _ => {} + } + } + } + if let Some(feature) = feature { + features.push((feature, since, attr.span)); + } + // We need to iterate over the other attributes, because + // `rustc_const_unstable` is not mutually exclusive with + // the other stability attributes, so we can't just `break` + // here. + } + } + } + + features + } + + fn collect_feature(&mut self, feature: Symbol, since: Option, span: Span) { + let already_in_stable = self.lib_features.stable.contains_key(&feature); + let already_in_unstable = self.lib_features.unstable.contains(&feature); + + match (since, already_in_stable, already_in_unstable) { + (Some(since), _, false) => { + self.lib_features.stable.insert(feature, since); + } + (None, false, _) => { + self.lib_features.unstable.insert(feature); + } + (Some(_), _, true) | (None, true, _) => { + let msg = format!( + "feature `{}` is declared {}, but was previously declared {}", + feature, + if since.is_some() { "stable"} else { "unstable" }, + if since.is_none() { "stable"} else { "unstable" }, + ); + self.tcx.sess.struct_span_err_with_code(span, &msg, + DiagnosticId::Error("E0711".into())).emit(); + } + } + } + + fn collect_from_attrs(&mut self, attrs: &[Attribute]) { + for (feature, stable, span) in self.extract(attrs) { + self.collect_feature(feature, stable, span); + } + } +} + +impl<'a, 'v, 'tcx> ItemLikeVisitor<'v> for LibFeatureCollector<'a, 'tcx> { + fn visit_item(&mut self, item: &hir::Item) { + self.collect_from_attrs(&item.attrs); + } + + fn visit_trait_item(&mut self, trait_item: &hir::TraitItem) { + self.collect_from_attrs(&trait_item.attrs); + } + + fn visit_impl_item(&mut self, impl_item: &hir::ImplItem) { + self.collect_from_attrs(&impl_item.attrs); + } +} + +pub fn collect<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> LibFeatures { + let mut collector = LibFeatureCollector::new(tcx); + for &cnum in tcx.crates().iter() { + for &(feature, since) in tcx.defined_lib_features(cnum).iter() { + collector.collect_feature(feature, since, DUMMY_SP); + } + } + collector.collect_from_attrs(&tcx.hir.krate().attrs); + tcx.hir.krate().visit_all_item_likes(&mut collector); + collector.lib_features +} diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 262a617cb69..75dbd70a982 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -813,37 +813,42 @@ pub fn check_unused_or_stable_features<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { krate.visit_all_item_likes(&mut missing.as_deep_visitor()); } - let ref declared_lib_features = tcx.features().declared_lib_features; - let mut remaining_lib_features: FxHashMap - = declared_lib_features.clone().into_iter().collect(); - remaining_lib_features.remove(&Symbol::intern("proc_macro")); - for &(ref stable_lang_feature, span) in &tcx.features().declared_stable_lang_features { - let version = find_lang_feature_accepted_version(&stable_lang_feature.as_str()) + let since = find_lang_feature_accepted_version(&stable_lang_feature.as_str()) .expect("unexpectedly couldn't find version feature was stabilized"); tcx.lint_node(lint::builtin::STABLE_FEATURES, ast::CRATE_NODE_ID, span, - &format_stable_since_msg(version)); - } - - // FIXME(#44232) the `used_features` table no longer exists, so we don't - // lint about unknown or unused features. We should reenable - // this one day! - // - // let index = tcx.stability(); - // for (used_lib_feature, level) in &index.used_features { - // remaining_lib_features.remove(used_lib_feature); - // } - // - // for &span in remaining_lib_features.values() { - // tcx.lint_node(lint::builtin::UNUSED_FEATURES, - // ast::CRATE_NODE_ID, - // span, - // "unused or unknown feature"); - // } + &format_stable_since_msg(*stable_lang_feature, since)); + } + + let ref declared_lib_features = tcx.features().declared_lib_features; + + let mut remaining_lib_features = FxHashMap(); + for (feature, span) in declared_lib_features.clone().into_iter() { + remaining_lib_features.insert(feature, span); + } + // FIXME(varkor): we don't properly handle lib features behind `cfg` attributes yet, + // but it happens just to affect `libc`, so we're just going to hard-code it for now. + remaining_lib_features.remove(&Symbol::intern("libc")); + + for (feature, stable) in tcx.lib_features().iter() { + remaining_lib_features.remove(&feature); + } + + for (feature, span) in remaining_lib_features { + tcx.lint_node(lint::builtin::UNKNOWN_FEATURES, + ast::CRATE_NODE_ID, + span, + &format!("unknown feature `{}`", feature)); + } + + // FIXME(#44232): the `used_features` table no longer exists, so we + // don't lint about unused features. We should reenable this one day! } -fn format_stable_since_msg(version: &str) -> String { - format!("this feature has been stable since {}. Attribute no longer needed", version) +fn format_stable_since_msg(feature: Symbol, since: &str) -> String { + // "this feature has been stable since {}. Attribute no longer needed" + format!("the feature `{}` has been stable since {} and no longer requires \ + an attribute to enable", feature, since) } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 0cfdea68e88..21f482ecf39 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1192,6 +1192,10 @@ pub fn consider_optimizing String>(&self, msg: T) -> bool { self.sess.consider_optimizing(&cname, msg) } + pub fn lib_features(self) -> Lrc { + self.get_lib_features(LOCAL_CRATE) + } + pub fn lang_items(self) -> Lrc { self.get_lang_items(LOCAL_CRATE) } @@ -2840,6 +2844,11 @@ pub fn provide(providers: &mut ty::query::Providers) { assert_eq!(id, LOCAL_CRATE); tcx.crate_name }; + providers.get_lib_features = |tcx, id| { + assert_eq!(id, LOCAL_CRATE); + // FIXME(#42293): see comment below. + tcx.dep_graph.with_ignore(|| Lrc::new(middle::lib_features::collect(tcx))) + }; providers.get_lang_items = |tcx, id| { assert_eq!(id, LOCAL_CRATE); // FIXME(#42293) Right now we insert a `with_ignore` node in the dep diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs index d2648cad55e..ecf35c1b0da 100644 --- a/src/librustc/ty/query/config.rs +++ b/src/librustc/ty/query/config.rs @@ -626,6 +626,18 @@ fn describe(_tcx: TyCtxt, _: CrateNum) -> String { } } +impl<'tcx> QueryDescription<'tcx> for queries::get_lib_features<'tcx> { + fn describe(_tcx: TyCtxt, _: CrateNum) -> String { + format!("calculating the lib features map") + } +} + +impl<'tcx> QueryDescription<'tcx> for queries::defined_lib_features<'tcx> { + fn describe(_tcx: TyCtxt, _: CrateNum) -> String { + format!("calculating the lib features defined in a crate") + } +} + impl<'tcx> QueryDescription<'tcx> for queries::get_lang_items<'tcx> { fn describe(_tcx: TyCtxt, _: CrateNum) -> String { "calculating the lang items map".to_string() diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index ab9bdd82e01..35080123d3e 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -24,6 +24,7 @@ use middle::region; use middle::resolve_lifetime::{ResolveLifetimes, Region, ObjectLifetimeDefault}; use middle::stability::{self, DeprecationEntry}; +use middle::lib_features::LibFeatures; use middle::lang_items::{LanguageItems, LangItem}; use middle::exported_symbols::{SymbolExportLevel, ExportedSymbol}; use mir::interpret::ConstEvalResult; @@ -492,6 +493,9 @@ [] fn item_children: ItemChildren(DefId) -> Lrc>, [] fn extern_mod_stmt_cnum: ExternModStmtCnum(DefId) -> Option, + [] fn get_lib_features: get_lib_features_node(CrateNum) -> Lrc, + [] fn defined_lib_features: DefinedLibFeatures(CrateNum) + -> Lrc)>>, [] fn get_lang_items: get_lang_items_node(CrateNum) -> Lrc, [] fn defined_lang_items: DefinedLangItems(CrateNum) -> Lrc>, [] fn missing_lang_items: MissingLangItems(CrateNum) -> Lrc>, @@ -800,6 +804,10 @@ fn link_args_node<'tcx>(_: CrateNum) -> DepConstructor<'tcx> { DepConstructor::LinkArgs } +fn get_lib_features_node<'tcx>(_: CrateNum) -> DepConstructor<'tcx> { + DepConstructor::GetLibFeatures +} + fn get_lang_items_node<'tcx>(_: CrateNum) -> DepConstructor<'tcx> { DepConstructor::GetLangItems } diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 0fa643d796e..f59e48cb351 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1218,6 +1218,8 @@ macro_rules! force { DepKind::CrateName => { force!(crate_name, krate!()); } DepKind::ItemChildren => { force!(item_children, def_id!()); } DepKind::ExternModStmtCnum => { force!(extern_mod_stmt_cnum, def_id!()); } + DepKind::GetLibFeatures => { force!(get_lib_features, LOCAL_CRATE); } + DepKind::DefinedLibFeatures => { force!(defined_lib_features, krate!()); } DepKind::GetLangItems => { force!(get_lang_items, LOCAL_CRATE); } DepKind::DefinedLangItems => { force!(defined_lang_items, krate!()); } DepKind::MissingLangItems => { force!(missing_lang_items, krate!()); } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index b92ac0ff57b..396e5e869f3 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -188,6 +188,7 @@ macro_rules! add_lint_group { UNUSED_DOC_COMMENTS, UNUSED_EXTERN_CRATES, UNUSED_FEATURES, + UNKNOWN_FEATURES, UNUSED_LABELS, UNUSED_PARENS); @@ -342,7 +343,6 @@ macro_rules! add_lint_group { store.register_renamed("bare_trait_object", "bare_trait_objects"); store.register_renamed("unstable_name_collision", "unstable_name_collisions"); store.register_renamed("unused_doc_comment", "unused_doc_comments"); - store.register_renamed("unknown_features", "unused_features"); store.register_removed("unsigned_negation", "replaced by negate_unsigned feature gate"); store.register_removed("negate_unsigned", "cast a signed value instead"); store.register_removed("raw_pointer_derive", "using derive with raw pointers is ok"); diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 916c0920e0b..060dddd5343 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -240,6 +240,7 @@ fn into_args(self) -> (DefId, DefId) { (self.0.as_def_id(), self.1) } cdata.each_child_of_item(def_id.index, |child| result.push(child), tcx.sess); Lrc::new(result) } + defined_lib_features => { Lrc::new(cdata.get_lib_features()) } defined_lang_items => { Lrc::new(cdata.get_lang_items()) } missing_lang_items => { Lrc::new(cdata.get_missing_lang_items()) } diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index ab566654c38..45a61ec3308 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -645,6 +645,14 @@ pub fn get_impl_trait(&self, self.get_impl_data(id).trait_ref.map(|tr| tr.decode((self, tcx))) } + /// Iterates over all the stability attributes in the given crate. + pub fn get_lib_features(&self) -> Vec<(ast::Name, Option)> { + self.root + .lib_features + .decode(self) + .collect() + } + /// Iterates over the language items in the given crate. pub fn get_lang_items(&self) -> Vec<(DefId, usize)> { self.root diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 4a02a278bc1..9bb5a849bc7 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -394,6 +394,11 @@ fn encode_crate_root(&mut self) -> Lazy { ()); let dep_bytes = self.position() - i; + // Encode the lib features. + i = self.position(); + let lib_features = self.tracked(IsolatedEncoder::encode_lib_features, ()); + let lib_feature_bytes = self.position() - i; + // Encode the language items. i = self.position(); let lang_items = self.tracked(IsolatedEncoder::encode_lang_items, ()); @@ -513,6 +518,7 @@ fn encode_crate_root(&mut self) -> Lazy { crate_deps, dylib_dependency_formats, + lib_features, lang_items, lang_items_missing, native_libraries, @@ -537,6 +543,7 @@ fn encode_crate_root(&mut self) -> Lazy { println!("metadata stats:"); println!(" dep bytes: {}", dep_bytes); + println!(" lib feature bytes: {}", lib_feature_bytes); println!(" lang item bytes: {}", lang_item_bytes); println!(" native bytes: {}", native_lib_bytes); println!(" codemap bytes: {}", codemap_bytes); @@ -1456,6 +1463,12 @@ fn encode_crate_deps(&mut self, _: ()) -> LazySeq { self.lazy_seq_ref(deps.iter().map(|&(_, ref dep)| dep)) } + fn encode_lib_features(&mut self, _: ()) -> LazySeq<(ast::Name, Option)> { + let tcx = self.tcx; + let lib_features = tcx.lib_features(); + self.lazy_seq(lib_features.iter()) + } + fn encode_lang_items(&mut self, _: ()) -> LazySeq<(DefIndex, usize)> { let tcx = self.tcx; let lang_items = tcx.lang_items(); diff --git a/src/librustc_metadata/schema.rs b/src/librustc_metadata/schema.rs index d7c54cbc81d..894c7cbf683 100644 --- a/src/librustc_metadata/schema.rs +++ b/src/librustc_metadata/schema.rs @@ -198,6 +198,7 @@ pub struct CrateRoot { pub crate_deps: LazySeq, pub dylib_dependency_formats: LazySeq>, + pub lib_features: LazySeq<(Symbol, Option)>, pub lang_items: LazySeq<(DefIndex, usize)>, pub lang_items_missing: LazySeq, pub native_libraries: LazySeq, diff --git a/src/libsyntax/diagnostic_list.rs b/src/libsyntax/diagnostic_list.rs index 8534969c623..20be7f8361a 100644 --- a/src/libsyntax/diagnostic_list.rs +++ b/src/libsyntax/diagnostic_list.rs @@ -374,6 +374,18 @@ fn main() {} "##, +E0635: r##" +The `#![feature]` attribute specified an unknown feature. + +Erroneous code example: + +```compile_fail,E0635 +#![feature(nonexistent_rust_feature)] // error: unknown feature +``` + +"##, + + } register_diagnostics! { -- 2.44.0