Auto merge of #47223 - alexcrichton:new-target-feature, r=eddyb

rustc: Tweak `#[target_feature]` syntax

This is an implementation of the `#[target_feature]` syntax-related changes of
[RFC 2045][rfc]. Notably two changes have been implemented:

* The new syntax is `#[target_feature(enable = "..")]` instead of
  `#[target_feature = "+.."]`. The `enable` key is necessary instead of the `+`
  to indicate that a feature is being enabled, and a sub-list is used for
  possible expansion in the future. Additionally within this syntax the feature
  names being enabled are now whitelisted against a known set of target feature
  names that we know about.

* The `#[target_feature]` attribute can only be applied to unsafe functions. It
  was decided in the RFC that invoking an instruction possibly not defined for
  the current processor is undefined behavior, so to enable this feature for now
  it requires an `unsafe` intervention.

[rfc]: https://github.com/rust-lang/rfcs/blob/master/text/2045-target-feature.md

Author: bors

src/librustc/dep_graph/dep_node.rs

@@ -634,6 +634,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 {

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,45 +29,51 @@ 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" {
                     self.check_inline(attr, item, target)
                 }
             }
         }
+
         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; }
             }
         }

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> {

src/librustc/ty/maps/mod.rs

@@ -360,6 +360,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<FxHashSet<String>>,
+    [] fn target_features_enabled: TargetFeaturesEnabled(DefId) -> Rc<Vec<String>>,
+
 }
 
 //////////////////////////////////////////////////////////////////////
@@ -505,3 +510,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
+}

src/librustc/ty/maps/plumbing.rs

@@ -917,6 +917,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

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));

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<String>,
+    target_features: &mut Vec<String>,
+) {
+    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();
+        }
+    }
+}

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();
 

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) {