Rollup merge of #64809 - davidtwco:issue-64768-target-feature-const, r=varkor

Centril · web-flow · commit 00cba5b092be · 2019-09-28T05:37:47.000+02:00
hir: Disallow `target_feature` on constants Fixes #64768. This PR fixes an ICE when `#[target_feature]` is applied to constants by disallowing this with the same error as when `#[target_feature]` is applied to other places it shouldn't be. I couldn't see anything in the [RFC](https://github.com/rust-lang/rfcs/blob/master/text/2045-target-feature.md) that suggested that `#[target_feature]` should be applicable to constants or any tests that suggested it should, though I might have missed something - if this is desirable in future, it remains possible to remove this error (but for the time being, I think this error is better than an ICE). I also added some extra cases to the test for other places where `#[target_feature]` should not be permitted. cc @gnzlbg
diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs
@@ -93,70 +93,102 @@ struct CheckAttrVisitor<'tcx> {
 impl CheckAttrVisitor<'tcx> {
     /// Checks any attribute.
     fn check_attributes(&self, item: &hir::Item, target: Target) {
-        if target == Target::Fn || target == Target::Const {
-            self.tcx.codegen_fn_attrs(self.tcx.hir().local_def_id(item.hir_id));
-        } else if let Some(a) = item.attrs.iter().find(|a| a.check_name(sym::target_feature)) {
-            self.tcx.sess.struct_span_err(a.span, "attribute should be applied to a function")
-                .span_label(item.span, "not a function")
-                .emit();
-        }
-
+        let mut is_valid = true;
         for attr in &item.attrs {
-            if attr.check_name(sym::inline) {
+            is_valid &= if attr.check_name(sym::inline) {
                 self.check_inline(attr, &item.span, target)
             } else if attr.check_name(sym::non_exhaustive) {
                 self.check_non_exhaustive(attr, item, target)
             } else if attr.check_name(sym::marker) {
                 self.check_marker(attr, item, target)
-            }
+            } else if attr.check_name(sym::target_feature) {
+                self.check_target_feature(attr, item, target)
+            } else {
+                true
+            };
+        }
+
+        if !is_valid {
+            return;
+        }
+
+        if target == Target::Fn {
+            self.tcx.codegen_fn_attrs(self.tcx.hir().local_def_id(item.hir_id));
         }
 
         self.check_repr(item, target);
         self.check_used(item, target);
     }
 
-    /// Checks if an `#[inline]` is applied to a function or a closure.
-    fn check_inline(&self, attr: &hir::Attribute, span: &Span, target: Target) {
+    /// Checks if an `#[inline]` is applied to a function or a closure. Returns `true` if valid.
+    fn check_inline(&self, attr: &hir::Attribute, span: &Span, target: Target) -> bool {
         if target != Target::Fn && target != Target::Closure {
             struct_span_err!(self.tcx.sess,
                              attr.span,
                              E0518,
                              "attribute should be applied to function or closure")
                 .span_label(*span, "not a function or closure")
                 .emit();
+            false
+        } else {
+            true
         }
     }
 
-    /// Checks if the `#[non_exhaustive]` attribute on an `item` is valid.
-    fn check_non_exhaustive(&self, attr: &hir::Attribute, item: &hir::Item, target: Target) {
+    /// Checks if the `#[non_exhaustive]` attribute on an `item` is valid. Returns `true` if valid.
+    fn check_non_exhaustive(
+        &self,
+        attr: &hir::Attribute,
+        item: &hir::Item,
+        target: Target,
+    ) -> bool {
         match target {
-            Target::Struct | Target::Enum => { /* Valid */ },
+            Target::Struct | Target::Enum => true,
             _ => {
                 struct_span_err!(self.tcx.sess,
                                  attr.span,
                                  E0701,
                                  "attribute can only be applied to a struct or enum")
                     .span_label(item.span, "not a struct or enum")
                     .emit();
-                return;
+                false
             }
         }
     }
 
-    /// Checks if the `#[marker]` attribute on an `item` is valid.
-    fn check_marker(&self, attr: &hir::Attribute, item: &hir::Item, target: Target) {
+    /// Checks if the `#[marker]` attribute on an `item` is valid. Returns `true` if valid.
+    fn check_marker(&self, attr: &hir::Attribute, item: &hir::Item, target: Target) -> bool {
         match target {
-            Target::Trait => { /* Valid */ },
+            Target::Trait => true,
             _ => {
                 self.tcx.sess
                     .struct_span_err(attr.span, "attribute can only be applied to a trait")
                     .span_label(item.span, "not a trait")
                     .emit();
-                return;
+                false
             }
         }
     }
 
+    /// Checks if the `#[target_feature]` attribute on `item` is valid. Returns `true` if valid.
+    fn check_target_feature(
+        &self,
+        attr: &hir::Attribute,
+        item: &hir::Item,
+        target: Target,
+    ) -> bool {
+        match target {
+            Target::Fn => true,
+            _ => {
+                self.tcx.sess
+                    .struct_span_err(attr.span, "attribute should be applied to a function")
+                    .span_label(item.span, "not a function")
+                    .emit();
+                false
+            },
+        }
+    }
+
     /// Checks if the `#[repr]` attributes on `item` are valid.
     fn check_repr(&self, item: &hir::Item, target: Target) {
         // Extract the names of all repr hints, e.g., [foo, bar, align] for:
diff --git a/src/test/ui/attributes/multiple-invalid.rs b/src/test/ui/attributes/multiple-invalid.rs
@@ -0,0 +1,10 @@
+// This test checks that all expected errors occur when there are multiple invalid attributes
+// on an item.
+
+#[inline]
+//~^ ERROR attribute should be applied to function or closure [E0518]
+#[target_feature(enable = "sse2")]
+//~^ ERROR attribute should be applied to a function
+const FOO: u8 = 0;
+
+fn main() { }
diff --git a/src/test/ui/attributes/multiple-invalid.stderr b/src/test/ui/attributes/multiple-invalid.stderr
@@ -0,0 +1,21 @@
+error[E0518]: attribute should be applied to function or closure
+  --> $DIR/multiple-invalid.rs:4:1
+   |
+LL | #[inline]
+   | ^^^^^^^^^
+...
+LL | const FOO: u8 = 0;
+   | ------------------ not a function or closure
+
+error: attribute should be applied to a function
+  --> $DIR/multiple-invalid.rs:6:1
+   |
+LL | #[target_feature(enable = "sse2")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |
+LL | const FOO: u8 = 0;
+   | ------------------ not a function
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0518`.
diff --git a/src/test/ui/target-feature-wrong.stderr b/src/test/ui/target-feature-wrong.stderr
diff --git a/src/test/ui/target-feature/gate.rs b/src/test/ui/target-feature/gate.rs
diff --git a/src/test/ui/target-feature/gate.stderr b/src/test/ui/target-feature/gate.stderr
@@ -1,5 +1,5 @@
 error[E0658]: the target feature `avx512bw` is currently unstable
-  --> $DIR/target-feature-gate.rs:30:18
+  --> $DIR/gate.rs:30:18
    |
 LL | #[target_feature(enable = "avx512bw")]
    |                  ^^^^^^^^^^^^^^^^^^^
diff --git a/src/test/ui/target-feature/invalid-attribute.rs b/src/test/ui/target-feature/invalid-attribute.rs
@@ -35,6 +35,31 @@ fn bar() {}
 mod another {}
 //~^ NOTE not a function
 
+#[target_feature(enable = "sse2")]
+//~^ ERROR attribute should be applied to a function
+const FOO: usize = 7;
+//~^ NOTE not a function
+
+#[target_feature(enable = "sse2")]
+//~^ ERROR attribute should be applied to a function
+struct Foo;
+//~^ NOTE not a function
+
+#[target_feature(enable = "sse2")]
+//~^ ERROR attribute should be applied to a function
+enum Bar { }
+//~^ NOTE not a function
+
+#[target_feature(enable = "sse2")]
+//~^ ERROR attribute should be applied to a function
+union Qux { f1: u16, f2: u16 }
+//~^ NOTE not a function
+
+#[target_feature(enable = "sse2")]
+//~^ ERROR attribute should be applied to a function
+trait Baz { }
+//~^ NOTE not a function
+
 #[inline(always)]
 //~^ ERROR: cannot use `#[inline(always)]`
 #[target_feature(enable = "sse2")]
diff --git a/src/test/ui/target-feature/invalid-attribute.stderr b/src/test/ui/target-feature/invalid-attribute.stderr
@@ -0,0 +1,95 @@
+error: malformed `target_feature` attribute input
+  --> $DIR/invalid-attribute.rs:16:1
+   |
+LL | #[target_feature = "+sse2"]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[target_feature(enable = "name")]`
+
+error: the feature named `foo` is not valid for this target
+  --> $DIR/invalid-attribute.rs:18:18
+   |
+LL | #[target_feature(enable = "foo")]
+   |                  ^^^^^^^^^^^^^^ `foo` is not valid for this target
+
+error: malformed `target_feature` attribute input
+  --> $DIR/invalid-attribute.rs:21:18
+   |
+LL | #[target_feature(bar)]
+   |                  ^^^ help: must be of the form: `enable = ".."`
+
+error: malformed `target_feature` attribute input
+  --> $DIR/invalid-attribute.rs:23:18
+   |
+LL | #[target_feature(disable = "baz")]
+   |                  ^^^^^^^^^^^^^^^ help: must be of the form: `enable = ".."`
+
+error: `#[target_feature(..)]` can only be applied to `unsafe` functions
+  --> $DIR/invalid-attribute.rs:27:1
+   |
+LL | #[target_feature(enable = "sse2")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can only be applied to `unsafe` functions
+...
+LL | fn bar() {}
+   | ----------- not an `unsafe` function
+
+error: attribute should be applied to a function
+  --> $DIR/invalid-attribute.rs:33:1
+   |
+LL | #[target_feature(enable = "sse2")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |
+LL | mod another {}
+   | -------------- not a function
+
+error: attribute should be applied to a function
+  --> $DIR/invalid-attribute.rs:38:1
+   |
+LL | #[target_feature(enable = "sse2")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |
+LL | const FOO: usize = 7;
+   | --------------------- not a function
+
+error: attribute should be applied to a function
+  --> $DIR/invalid-attribute.rs:43:1
+   |
+LL | #[target_feature(enable = "sse2")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |
+LL | struct Foo;
+   | ----------- not a function
+
+error: attribute should be applied to a function
+  --> $DIR/invalid-attribute.rs:48:1
+   |
+LL | #[target_feature(enable = "sse2")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |
+LL | enum Bar { }
+   | ------------ not a function
+
+error: attribute should be applied to a function
+  --> $DIR/invalid-attribute.rs:53:1
+   |
+LL | #[target_feature(enable = "sse2")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |
+LL | union Qux { f1: u16, f2: u16 }
+   | ------------------------------ not a function
+
+error: attribute should be applied to a function
+  --> $DIR/invalid-attribute.rs:58:1
+   |
+LL | #[target_feature(enable = "sse2")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |
+LL | trait Baz { }
+   | ------------- not a function
+
+error: cannot use `#[inline(always)]` with `#[target_feature]`
+  --> $DIR/invalid-attribute.rs:63:1
+   |
+LL | #[inline(always)]
+   | ^^^^^^^^^^^^^^^^^
+
+error: aborting due to 12 previous errors
+

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	error[E0658]: the target feature `avx512bw` is currently unstable
`2`		`- --> $DIR/target-feature-gate.rs:30:18`
	`2`	`+ --> $DIR/gate.rs:30:18`
`3`	`3`	`\|`
`4`	`4`	`LL \| #[target_feature(enable = "avx512bw")]`
`5`	`5`	`\| ^^^^^^^^^^^^^^^^^^^`