diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index ec52c6cf57bf7..eda7ddb161f12 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -635,6 +635,10 @@ define_dep_nodes!( <'tcx> [] Null, [] SubstituteNormalizeAndTestPredicates { key: (DefId, &'tcx Substs<'tcx>) }, + + [input] TargetFeaturesWhitelist, + [] TargetFeaturesEnabled(DefId), + ); trait DepNodeParams<'a, 'gcx: 'tcx + 'a, 'tcx: 'a> : fmt::Debug { diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs index 7792726006859..4b528a0fdc778 100644 --- a/src/librustc/hir/check_attr.rs +++ b/src/librustc/hir/check_attr.rs @@ -14,11 +14,10 @@ //! conflicts between multiple such attributes attached to the same //! item. -use session::Session; +use ty::TyCtxt; -use syntax::ast; -use syntax::visit; -use syntax::visit::Visitor; +use hir; +use hir::intravisit::{self, Visitor, NestedVisitorMap}; #[derive(Copy, Clone, PartialEq)] enum Target { @@ -30,24 +29,26 @@ enum Target { } impl Target { - fn from_item(item: &ast::Item) -> Target { + fn from_item(item: &hir::Item) -> Target { match item.node { - ast::ItemKind::Fn(..) => Target::Fn, - ast::ItemKind::Struct(..) => Target::Struct, - ast::ItemKind::Union(..) => Target::Union, - ast::ItemKind::Enum(..) => Target::Enum, + hir::ItemFn(..) => Target::Fn, + hir::ItemStruct(..) => Target::Struct, + hir::ItemUnion(..) => Target::Union, + hir::ItemEnum(..) => Target::Enum, _ => Target::Other, } } } -struct CheckAttrVisitor<'a> { - sess: &'a Session, +struct CheckAttrVisitor<'a, 'tcx: 'a> { + tcx: TyCtxt<'a, 'tcx, 'tcx>, } -impl<'a> CheckAttrVisitor<'a> { +impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { /// Check any attribute. - fn check_attributes(&self, item: &ast::Item, target: Target) { + fn check_attributes(&self, item: &hir::Item, target: Target) { + self.tcx.target_features_enabled(self.tcx.hir.local_def_id(item.id)); + for attr in &item.attrs { if let Some(name) = attr.name() { if name == "inline" { @@ -55,20 +56,24 @@ impl<'a> CheckAttrVisitor<'a> { } } } + self.check_repr(item, target); } /// Check if an `#[inline]` is applied to a function. - fn check_inline(&self, attr: &ast::Attribute, item: &ast::Item, target: Target) { + fn check_inline(&self, attr: &hir::Attribute, item: &hir::Item, target: Target) { if target != Target::Fn { - struct_span_err!(self.sess, attr.span, E0518, "attribute should be applied to function") + struct_span_err!(self.tcx.sess, + attr.span, + E0518, + "attribute should be applied to function") .span_label(item.span, "not a function") .emit(); } } /// Check if the `#[repr]` attributes on `item` are valid. - fn check_repr(&self, item: &ast::Item, target: Target) { + fn check_repr(&self, item: &hir::Item, target: Target) { // Extract the names of all repr hints, e.g., [foo, bar, align] for: // ``` // #[repr(foo)] @@ -144,7 +149,7 @@ impl<'a> CheckAttrVisitor<'a> { } _ => continue, }; - struct_span_err!(self.sess, hint.span, E0517, + struct_span_err!(self.tcx.sess, hint.span, E0517, "attribute should be applied to {}", allowed_targets) .span_label(item.span, format!("not {} {}", article, allowed_targets)) .emit(); @@ -154,32 +159,37 @@ impl<'a> CheckAttrVisitor<'a> { if (int_reprs > 1) || (is_simd && is_c) || (int_reprs == 1 && is_c && is_c_like_enum(item)) { - // Just point at all repr hints. This is not ideal, but tracking precisely which ones - // are at fault is a huge hassle. + // Just point at all repr hints. This is not ideal, but tracking + // precisely which ones are at fault is a huge hassle. let spans: Vec<_> = hints.iter().map(|hint| hint.span).collect(); - span_warn!(self.sess, spans, E0566, + span_warn!(self.tcx.sess, spans, E0566, "conflicting representation hints"); } } } -impl<'a> Visitor<'a> for CheckAttrVisitor<'a> { - fn visit_item(&mut self, item: &'a ast::Item) { +impl<'a, 'tcx> Visitor<'tcx> for CheckAttrVisitor<'a, 'tcx> { + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + NestedVisitorMap::None + } + + fn visit_item(&mut self, item: &'tcx hir::Item) { let target = Target::from_item(item); self.check_attributes(item, target); - visit::walk_item(self, item); + intravisit::walk_item(self, item); } } -pub fn check_crate(sess: &Session, krate: &ast::Crate) { - visit::walk_crate(&mut CheckAttrVisitor { sess: sess }, krate); +pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { + let mut checker = CheckAttrVisitor { tcx }; + tcx.hir.krate().visit_all_item_likes(&mut checker.as_deep_visitor()); } -fn is_c_like_enum(item: &ast::Item) -> bool { - if let ast::ItemKind::Enum(ref def, _) = item.node { +fn is_c_like_enum(item: &hir::Item) -> bool { + if let hir::ItemEnum(ref def, _) = item.node { for variant in &def.variants { match variant.node.data { - ast::VariantData::Unit(_) => { /* continue */ } + hir::VariantData::Unit(_) => { /* continue */ } _ => { return false; } } } diff --git a/src/librustc/ty/maps/config.rs b/src/librustc/ty/maps/config.rs index 881c59e05aac6..8dedcb24c2fb6 100644 --- a/src/librustc/ty/maps/config.rs +++ b/src/librustc/ty/maps/config.rs @@ -631,6 +631,12 @@ impl<'tcx> QueryDescription<'tcx> for queries::substitute_normalize_and_test_pre } } +impl<'tcx> QueryDescription<'tcx> for queries::target_features_whitelist<'tcx> { + fn describe(_tcx: TyCtxt, _: CrateNum) -> String { + format!("looking up the whitelist of target features") + } +} + macro_rules! impl_disk_cacheable_query( ($query_name:ident, |$key:tt| $cond:expr) => { impl<'tcx> QueryDescription<'tcx> for queries::$query_name<'tcx> { diff --git a/src/librustc/ty/maps/mod.rs b/src/librustc/ty/maps/mod.rs index bd8c22aab9316..51cae9e13f883 100644 --- a/src/librustc/ty/maps/mod.rs +++ b/src/librustc/ty/maps/mod.rs @@ -363,6 +363,11 @@ define_maps! { <'tcx> [] fn substitute_normalize_and_test_predicates: substitute_normalize_and_test_predicates_node((DefId, &'tcx Substs<'tcx>)) -> bool, + + [] fn target_features_whitelist: + target_features_whitelist_node(CrateNum) -> Rc>, + [] fn target_features_enabled: TargetFeaturesEnabled(DefId) -> Rc>, + } ////////////////////////////////////////////////////////////////////// @@ -508,3 +513,7 @@ fn substitute_normalize_and_test_predicates_node<'tcx>(key: (DefId, &'tcx Substs -> DepConstructor<'tcx> { DepConstructor::SubstituteNormalizeAndTestPredicates { key } } + +fn target_features_whitelist_node<'tcx>(_: CrateNum) -> DepConstructor<'tcx> { + DepConstructor::TargetFeaturesWhitelist +} diff --git a/src/librustc/ty/maps/plumbing.rs b/src/librustc/ty/maps/plumbing.rs index dd8b8a2e5da82..4f6f1ab0ee2dd 100644 --- a/src/librustc/ty/maps/plumbing.rs +++ b/src/librustc/ty/maps/plumbing.rs @@ -918,6 +918,9 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>, } DepKind::IsTranslatedFunction => { force!(is_translated_function, def_id!()); } DepKind::OutputFilenames => { force!(output_filenames, LOCAL_CRATE); } + + DepKind::TargetFeaturesWhitelist => { force!(target_features_whitelist, LOCAL_CRATE); } + DepKind::TargetFeaturesEnabled => { force!(target_features_enabled, def_id!()); } } true diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index b726576220801..73c1b698087fc 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -210,10 +210,6 @@ pub fn compile_input(sess: &Session, Ok(())); } - time(sess.time_passes(), "attribute checking", || { - hir::check_attr::check_crate(sess, &expanded_crate); - }); - let opt_crate = if control.keep_ast { Some(&expanded_crate) } else { @@ -1038,6 +1034,10 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(control: &CompileController, // tcx available. rustc_incremental::dep_graph_tcx_init(tcx); + time(sess.time_passes(), "attribute checking", || { + hir::check_attr::check_crate(tcx) + }); + time(time_passes, "stability checking", || stability::check_unstable_api_usage(tcx)); diff --git a/src/librustc_trans/attributes.rs b/src/librustc_trans/attributes.rs index 745aa0da82900..f3105e03523a3 100644 --- a/src/librustc_trans/attributes.rs +++ b/src/librustc_trans/attributes.rs @@ -10,11 +10,18 @@ //! Set and unset common attributes on LLVM values. use std::ffi::{CStr, CString}; +use std::rc::Rc; +use rustc::hir::Unsafety; +use rustc::hir::def_id::{DefId, LOCAL_CRATE}; use rustc::session::config::Sanitizer; +use rustc::ty::TyCtxt; +use rustc::ty::maps::Providers; +use rustc_data_structures::fx::FxHashSet; use llvm::{self, Attribute, ValueRef}; use llvm::AttributePlace::Function; +use llvm_util; pub use syntax::attr::{self, InlineAttr}; use syntax::ast; use context::CrateContext; @@ -94,23 +101,16 @@ pub fn set_probestack(ccx: &CrateContext, llfn: ValueRef) { /// Composite function which sets LLVM attributes for function depending on its AST (#[attribute]) /// attributes. -pub fn from_fn_attrs(ccx: &CrateContext, attrs: &[ast::Attribute], llfn: ValueRef) { +pub fn from_fn_attrs(ccx: &CrateContext, llfn: ValueRef, id: DefId) { use syntax::attr::*; - inline(llfn, find_inline_attr(Some(ccx.sess().diagnostic()), attrs)); + let attrs = ccx.tcx().get_attrs(id); + inline(llfn, find_inline_attr(Some(ccx.sess().diagnostic()), &attrs)); set_frame_pointer_elimination(ccx, llfn); set_probestack(ccx, llfn); - let mut target_features = vec![]; - for attr in attrs { - if attr.check_name("target_feature") { - if let Some(val) = attr.value_str() { - for feat in val.as_str().split(",").map(|f| f.trim()) { - if !feat.is_empty() && !feat.contains('\0') { - target_features.push(feat.to_string()); - } - } - } - } else if attr.check_name("cold") { + + for attr in attrs.iter() { + if attr.check_name("cold") { Attribute::Cold.apply_llfn(Function, llfn); } else if attr.check_name("naked") { naked(llfn, true); @@ -123,6 +123,8 @@ pub fn from_fn_attrs(ccx: &CrateContext, attrs: &[ast::Attribute], llfn: ValueRe unwind(llfn, false); } } + + let target_features = ccx.tcx().target_features_enabled(id); if !target_features.is_empty() { let val = CString::new(target_features.join(",")).unwrap(); llvm::AddFunctionAttrStringValue( @@ -134,3 +136,97 @@ pub fn from_fn_attrs(ccx: &CrateContext, attrs: &[ast::Attribute], llfn: ValueRe fn cstr(s: &'static str) -> &CStr { CStr::from_bytes_with_nul(s.as_bytes()).expect("null-terminated string") } + +pub fn provide(providers: &mut Providers) { + providers.target_features_whitelist = |tcx, cnum| { + assert_eq!(cnum, LOCAL_CRATE); + Rc::new(llvm_util::target_feature_whitelist(tcx.sess) + .iter() + .map(|c| c.to_str().unwrap().to_string()) + .collect()) + }; + + providers.target_features_enabled = |tcx, id| { + let whitelist = tcx.target_features_whitelist(LOCAL_CRATE); + let mut target_features = Vec::new(); + for attr in tcx.get_attrs(id).iter() { + if !attr.check_name("target_feature") { + continue + } + if let Some(val) = attr.value_str() { + for feat in val.as_str().split(",").map(|f| f.trim()) { + if !feat.is_empty() && !feat.contains('\0') { + target_features.push(feat.to_string()); + } + } + let msg = "#[target_feature = \"..\"] is deprecated and will \ + eventually be removed, use \ + #[target_feature(enable = \"..\")] instead"; + tcx.sess.span_warn(attr.span, &msg); + continue + } + + if tcx.fn_sig(id).unsafety() == Unsafety::Normal { + let msg = "#[target_feature(..)] can only be applied to \ + `unsafe` function"; + tcx.sess.span_err(attr.span, msg); + } + from_target_feature(tcx, attr, &whitelist, &mut target_features); + } + Rc::new(target_features) + }; +} + +fn from_target_feature( + tcx: TyCtxt, + attr: &ast::Attribute, + whitelist: &FxHashSet, + target_features: &mut Vec, +) { + let list = match attr.meta_item_list() { + Some(list) => list, + None => { + let msg = "#[target_feature] attribute must be of the form \ + #[target_feature(..)]"; + tcx.sess.span_err(attr.span, &msg); + return + } + }; + + for item in list { + if !item.check_name("enable") { + let msg = "#[target_feature(..)] only accepts sub-keys of `enable` \ + currently"; + tcx.sess.span_err(item.span, &msg); + continue + } + let value = match item.value_str() { + Some(list) => list, + None => { + let msg = "#[target_feature] attribute must be of the form \ + #[target_feature(enable = \"..\")]"; + tcx.sess.span_err(item.span, &msg); + continue + } + }; + let value = value.as_str(); + for feature in value.split(',') { + if whitelist.contains(feature) { + target_features.push(format!("+{}", feature)); + continue + } + + let msg = format!("the feature named `{}` is not valid for \ + this target", feature); + let mut err = tcx.sess.struct_span_err(item.span, &msg); + + if feature.starts_with("+") { + let valid = whitelist.contains(&feature[1..]); + if valid { + err.help("consider removing the leading `+` in the feature name"); + } + } + err.emit(); + } + } +} diff --git a/src/librustc_trans/callee.rs b/src/librustc_trans/callee.rs index 0a0f2615a1bd1..ccbc6620ffe09 100644 --- a/src/librustc_trans/callee.rs +++ b/src/librustc_trans/callee.rs @@ -99,8 +99,7 @@ pub fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, if instance.def.is_inline(tcx) { attributes::inline(llfn, attributes::InlineAttr::Hint); } - let attrs = instance.def.attrs(ccx.tcx()); - attributes::from_fn_attrs(ccx, &attrs, llfn); + attributes::from_fn_attrs(ccx, llfn, instance.def.def_id()); let instance_def_id = instance.def_id(); diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs index ee08a7f1ec471..974c268749b85 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -167,6 +167,7 @@ impl rustc_trans_utils::trans_crate::TransCrate for LlvmTransCrate { back::symbol_names::provide(providers); back::symbol_export::provide(providers); base::provide(providers); + attributes::provide(providers); } fn provide_extern(providers: &mut ty::maps::Providers) { diff --git a/src/librustc_trans/llvm_util.rs b/src/librustc_trans/llvm_util.rs index b3d0b574d1d43..8112a9eeab1ef 100644 --- a/src/librustc_trans/llvm_util.rs +++ b/src/librustc_trans/llvm_util.rs @@ -13,8 +13,8 @@ use back::write::create_target_machine; use llvm; use rustc::session::Session; use rustc::session::config::PrintRequest; -use libc::{c_int, c_char}; -use std::ffi::CString; +use libc::c_int; +use std::ffi::{CStr, CString}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Once; @@ -97,8 +97,18 @@ const POWERPC_WHITELIST: &'static [&'static str] = &["altivec\0", const MIPS_WHITELIST: &'static [&'static str] = &["msa\0"]; pub fn target_features(sess: &Session) -> Vec { + let whitelist = target_feature_whitelist(sess); let target_machine = create_target_machine(sess); + let mut features = Vec::new(); + for feat in whitelist { + if unsafe { llvm::LLVMRustHasFeature(target_machine, feat.as_ptr()) } { + features.push(Symbol::intern(feat.to_str().unwrap())); + } + } + features +} +pub fn target_feature_whitelist(sess: &Session) -> Vec<&CStr> { let whitelist = match &*sess.target.target.arch { "arm" => ARM_WHITELIST, "aarch64" => AARCH64_WHITELIST, @@ -108,15 +118,9 @@ pub fn target_features(sess: &Session) -> Vec { "powerpc" | "powerpc64" => POWERPC_WHITELIST, _ => &[], }; - - let mut features = Vec::new(); - for feat in whitelist { - assert_eq!(feat.chars().last(), Some('\0')); - if unsafe { llvm::LLVMRustHasFeature(target_machine, feat.as_ptr() as *const c_char) } { - features.push(Symbol::intern(&feat[..feat.len() - 1])); - } - } - features + whitelist.iter().map(|m| { + CStr::from_bytes_with_nul(m.as_bytes()).unwrap() + }).collect() } pub fn print_version() { diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index 31d8e092c4ae4..fa6a42e062d9f 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -199,7 +199,7 @@ fn predefine_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, if instance.def.is_inline(ccx.tcx()) { attributes::inline(lldecl, attributes::InlineAttr::Hint); } - attributes::from_fn_attrs(ccx, &attrs, lldecl); + attributes::from_fn_attrs(ccx, lldecl, instance.def.def_id()); ccx.instances().borrow_mut().insert(instance, lldecl); } diff --git a/src/test/ui/feature-gate/issue-43106-gating-of-inline.stderr b/src/test/ui/feature-gate/issue-43106-gating-of-inline.stderr index 92bda4d0446d9..444c4176994c5 100644 --- a/src/test/ui/feature-gate/issue-43106-gating-of-inline.stderr +++ b/src/test/ui/feature-gate/issue-43106-gating-of-inline.stderr @@ -1,3 +1,5 @@ +error[E0601]: main function not found + error[E0518]: attribute should be applied to function --> $DIR/issue-43106-gating-of-inline.rs:21:1 | @@ -37,7 +39,5 @@ error[E0518]: attribute should be applied to function 35 | #[inline = "2100"] impl S { } | ^^^^^^^^^^^^^^^^^^ ---------- not a function -error[E0601]: main function not found - error: aborting due to 6 previous errors diff --git a/src/test/ui/target-feature-wrong.rs b/src/test/ui/target-feature-wrong.rs new file mode 100644 index 0000000000000..e70d549ed573a --- /dev/null +++ b/src/test/ui/target-feature-wrong.rs @@ -0,0 +1,37 @@ +// Copyright 2015 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. + +// ignore-arm +// ignore-aarch64 +// ignore-wasm +// ignore-emscripten + +#![feature(target_feature)] + +#[target_feature = "+sse2"] +//~^ WARN: deprecated +#[target_feature(enable = "foo")] +//~^ ERROR: not valid for this target +#[target_feature(bar)] +//~^ ERROR: only accepts sub-keys +#[target_feature(disable = "baz")] +//~^ ERROR: only accepts sub-keys +unsafe fn foo() {} + +#[target_feature(enable = "sse2")] +//~^ ERROR: can only be applied to `unsafe` function +fn bar() {} + +fn main() { + unsafe { + foo(); + bar(); + } +} diff --git a/src/test/ui/target-feature-wrong.stderr b/src/test/ui/target-feature-wrong.stderr new file mode 100644 index 0000000000000..c5534bf147d5c --- /dev/null +++ b/src/test/ui/target-feature-wrong.stderr @@ -0,0 +1,32 @@ +warning: #[target_feature = ".."] is deprecated and will eventually be removed, use #[target_feature(enable = "..")] instead + --> $DIR/target-feature-wrong.rs:18:1 + | +18 | #[target_feature = "+sse2"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: the feature named `foo` is not valid for this target + --> $DIR/target-feature-wrong.rs:20:18 + | +20 | #[target_feature(enable = "foo")] + | ^^^^^^^^^^^^^^ + +error: #[target_feature(..)] only accepts sub-keys of `enable` currently + --> $DIR/target-feature-wrong.rs:22:18 + | +22 | #[target_feature(bar)] + | ^^^ + +error: #[target_feature(..)] only accepts sub-keys of `enable` currently + --> $DIR/target-feature-wrong.rs:24:18 + | +24 | #[target_feature(disable = "baz")] + | ^^^^^^^^^^^^^^^ + +error: #[target_feature(..)] can only be applied to `unsafe` function + --> $DIR/target-feature-wrong.rs:28:1 + | +28 | #[target_feature(enable = "sse2")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors +