feature error span on attr. for fn_must_use, SIMD/align, macro reëxport

zackmdavis · zackmdavis · commit 8bb29465d0cb · 2017-08-28T00:58:41.000-07:00
There were several feature-gated attributes for which the
feature-not-available error spans would point to the item annotated with
the gated attribute, when it would make more sense for the span to point
to the attribute itself: if the attribute is removed, the
function/struct/&amp;c. likely still makes sense and the program will
compile. (Note that we decline to make the analogous change for the
`main`, `start`, and `plugin_registrar` features, for in those cases it
makes sense for the span to implicate the entire function, of which
there is little hope of using without the gated attribute.)
diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs
@@ -490,6 +490,10 @@ pub fn contains_name(attrs: &[Attribute], name: &str) -> bool {
     })
 }
 
+pub fn find_by_name<'a>(attrs: &'a [Attribute], name: &str) -> Option<&'a Attribute> {
+    attrs.iter().find(|attr| attr.check_name(name))
+}
+
 pub fn first_attr_value_str_by_name(attrs: &[Attribute], name: &str) -> Option<Symbol> {
     attrs.iter()
         .find(|at| at.check_name(name))
diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs
@@ -1248,8 +1248,8 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
     fn visit_item(&mut self, i: &'a ast::Item) {
         match i.node {
             ast::ItemKind::ExternCrate(_) => {
-                if attr::contains_name(&i.attrs[..], "macro_reexport") {
-                    gate_feature_post!(&self, macro_reexport, i.span,
+                if let Some(attr) = attr::find_by_name(&i.attrs[..], "macro_reexport") {
+                    gate_feature_post!(&self, macro_reexport, attr.span,
                                        "macros reexports are experimental \
                                         and possibly buggy");
                 }
@@ -1276,36 +1276,32 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
                                         function may change over time, for now \
                                         a top-level `fn main()` is required");
                 }
-                if attr::contains_name(&i.attrs[..], "must_use") {
-                    gate_feature_post!(&self, fn_must_use, i.span,
+                if let Some(attr) = attr::find_by_name(&i.attrs[..], "must_use") {
+                    gate_feature_post!(&self, fn_must_use, attr.span,
                                        "`#[must_use]` on functions is experimental",
                                        GateStrength::Soft);
                 }
             }
 
             ast::ItemKind::Struct(..) => {
-                if attr::contains_name(&i.attrs[..], "simd") {
-                    gate_feature_post!(&self, simd, i.span,
+                if let Some(attr) = attr::find_by_name(&i.attrs[..], "simd") {
+                    gate_feature_post!(&self, simd, attr.span,
                                        "SIMD types are experimental and possibly buggy");
-                    self.context.parse_sess.span_diagnostic.span_warn(i.span,
+                    self.context.parse_sess.span_diagnostic.span_warn(attr.span,
                                                                       "the `#[simd]` attribute \
                                                                        is deprecated, use \
                                                                        `#[repr(simd)]` instead");
                 }
-                for attr in &i.attrs {
-                    if attr.path == "repr" {
-                        for item in attr.meta_item_list().unwrap_or_else(Vec::new) {
-                            if item.check_name("simd") {
-                                gate_feature_post!(&self, repr_simd, i.span,
-                                                   "SIMD types are experimental \
-                                                    and possibly buggy");
-
-                            }
-                            if item.check_name("align") {
-                                gate_feature_post!(&self, repr_align, i.span,
-                                                   "the struct `#[repr(align(u16))]` attribute \
-                                                    is experimental");
-                            }
+                if let Some(attr) = attr::find_by_name(&i.attrs[..], "repr") {
+                    for item in attr.meta_item_list().unwrap_or_else(Vec::new) {
+                        if item.check_name("simd") {
+                            gate_feature_post!(&self, repr_simd, attr.span,
+                                               "SIMD types are experimental and possibly buggy");
+                        }
+                        if item.check_name("align") {
+                            gate_feature_post!(&self, repr_align, attr.span,
+                                               "the struct `#[repr(align(u16))]` attribute \
+                                                is experimental");
                         }
                     }
                 }
@@ -1334,8 +1330,8 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
 
                 for impl_item in impl_items {
                     if let ast::ImplItemKind::Method(..) = impl_item.node {
-                        if attr::contains_name(&impl_item.attrs[..], "must_use") {
-                            gate_feature_post!(&self, fn_must_use, impl_item.span,
+                        if let Some(attr) = attr::find_by_name(&impl_item.attrs[..], "must_use") {
+                            gate_feature_post!(&self, fn_must_use, attr.span,
                                                "`#[must_use]` on methods is experimental",
                                                GateStrength::Soft);
                         }
diff --git a/src/test/compile-fail-fulldeps/gated-macro-reexports.rs b/src/test/compile-fail-fulldeps/gated-macro-reexports.rs
@@ -16,6 +16,6 @@
 #![crate_type = "dylib"]
 
 #[macro_reexport(reexported)]
+//~^ ERROR macros reexports are experimental and possibly buggy
 #[macro_use] #[no_link]
 extern crate macro_reexport_1;
-//~^ ERROR macros reexports are experimental and possibly buggy
diff --git a/src/test/compile-fail/feature-gate-fn_must_use.rs b/src/test/compile-fail/feature-gate-fn_must_use.rs
@@ -13,12 +13,12 @@
 struct MyStruct;
 
 impl MyStruct {
-    #[must_use]
-    fn need_to_use_method() -> bool { true } //~ WARN `#[must_use]` on methods is experimental
+    #[must_use] //~ WARN `#[must_use]` on methods is experimental
+    fn need_to_use_method() -> bool { true }
 }
 
-#[must_use]
-fn need_to_use_it() -> bool { true } //~ WARN `#[must_use]` on functions is experimental
+#[must_use] //~ WARN `#[must_use]` on functions is experimental
+fn need_to_use_it() -> bool { true }
 
 
 // Feature gates are tidy-required to have a specially named (or
diff --git a/src/test/compile-fail/feature-gate-repr-simd.rs b/src/test/compile-fail/feature-gate-repr-simd.rs
@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#[repr(simd)]
-struct Foo(u64, u64); //~ error: SIMD types are experimental
+#[repr(simd)] //~ error: SIMD types are experimental
+struct Foo(u64, u64);
 
 fn main() {}
diff --git a/src/test/compile-fail/feature-gate-repr_align.rs b/src/test/compile-fail/feature-gate-repr_align.rs
@@ -9,7 +9,7 @@
 // except according to those terms.
 #![feature(attr_literals)]
 
-#[repr(align(64))]
-struct Foo(u64, u64); //~ error: the struct `#[repr(align(u16))]` attribute is experimental
+#[repr(align(64))] //~ error: the struct `#[repr(align(u16))]` attribute is experimental
+struct Foo(u64, u64);
 
 fn main() {}
diff --git a/src/test/compile-fail/feature-gate-simd.rs b/src/test/compile-fail/feature-gate-simd.rs
@@ -11,13 +11,12 @@
 
 // pretty-expanded FIXME #23616
 
-#[repr(simd)]
+#[repr(simd)] //~ ERROR SIMD types are experimental
 struct RGBA {
     r: f32,
     g: f32,
     b: f32,
     a: f32
 }
-//~^^^^^^ ERROR SIMD types are experimental and possibly buggy (see issue #27731)
 
 pub fn main() {}
diff --git a/src/test/compile-fail/feature-gate/issue-43106-gating-of-builtin-attrs.rs b/src/test/compile-fail/feature-gate/issue-43106-gating-of-builtin-attrs.rs
@@ -354,8 +354,7 @@ mod repr {
     #[repr = "3900"] fn f() { }
     //~^ WARN unused attribute
 
-    #[repr = "3900"] struct S;
-    //~^ WARN unused attribute
+    struct S;
 
     #[repr = "3900"] type T = S;
     //~^ WARN unused attribute
diff --git a/src/test/ui/span/gated-features-attr-spans.rs b/src/test/ui/span/gated-features-attr-spans.rs
@@ -0,0 +1,39 @@
+// Copyright 2017 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.
+
+#![feature(attr_literals)]
+
+#[repr(align(16))]
+struct Gem {
+    mohs_hardness: u8,
+    poofed: bool,
+    weapon: Weapon,
+}
+
+#[repr(simd)]
+struct Weapon {
+    name: String,
+    damage: u32
+}
+
+impl Gem {
+    #[must_use] fn summon_weapon(&self) -> Weapon { self.weapon }
+}
+
+#[must_use]
+fn bubble(gem: Gem) -> Result<Gem, ()> {
+    if gem.poofed {
+        Ok(gem)
+    } else {
+        Err(())
+    }
+}
+
+fn main() {}
diff --git a/src/test/ui/span/gated-features-attr-spans.stderr b/src/test/ui/span/gated-features-attr-spans.stderr
@@ -0,0 +1,34 @@
+error: the struct `#[repr(align(u16))]` attribute is experimental (see issue #33626)
+  --> $DIR/gated-features-attr-spans.rs:13:1
+   |
+13 | #[repr(align(16))]
+   | ^^^^^^^^^^^^^^^^^^
+   |
+   = help: add #![feature(repr_align)] to the crate attributes to enable
+
+error: SIMD types are experimental and possibly buggy (see issue #27731)
+  --> $DIR/gated-features-attr-spans.rs:20:1
+   |
+20 | #[repr(simd)]
+   | ^^^^^^^^^^^^^
+   |
+   = help: add #![feature(repr_simd)] to the crate attributes to enable
+
+warning: `#[must_use]` on methods is experimental (see issue #43302)
+  --> $DIR/gated-features-attr-spans.rs:27:5
+   |
+27 |     #[must_use] fn summon_weapon(&self) -> Weapon { self.weapon }
+   |     ^^^^^^^^^^^
+   |
+   = help: add #![feature(fn_must_use)] to the crate attributes to enable
+
+warning: `#[must_use]` on functions is experimental (see issue #43302)
+  --> $DIR/gated-features-attr-spans.rs:30:1
+   |
+30 | #[must_use]
+   | ^^^^^^^^^^^
+   |
+   = help: add #![feature(fn_must_use)] to the crate attributes to enable
+
+error: aborting due to 2 previous errors
+
