]> git.lizzy.rs Git - rust.git/commitdiff
Check all repr hints together when checking for mis-applied attributes
authorRobin Kruppe <robin.kruppe@gmail.com>
Mon, 1 Jan 2018 20:42:12 +0000 (21:42 +0100)
committerRobin Kruppe <robin.kruppe@gmail.com>
Mon, 1 Jan 2018 21:05:29 +0000 (22:05 +0100)
Fixes #47094

Besides fixing that bug, this change has a user-visible effect on the spans in the "incompatible repr hints" warning and another error: they now point at `foo` and/or `bar` in `repr(foo, bar)` instead of the whole attribute. This is sometimes more precise (e.g., `#[repr(C, packed)]` on an enum points at the `packed`) but sometimes not. I moved a compile-fail test to a ui test to illustrate how it now looks in the common case of only one attribute.

src/librustc/hir/check_attr.rs
src/test/compile-fail/attr-usage-repr.rs [deleted file]
src/test/ui/attr-usage-repr.rs [new file with mode: 0644]
src/test/ui/attr-usage-repr.stderr [new file with mode: 0644]
src/test/ui/feature-gate-simd-ffi.rs
src/test/ui/feature-gate-simd-ffi.stderr
src/test/ui/issue-47094.rs [new file with mode: 0644]
src/test/ui/issue-47094.stderr [new file with mode: 0644]

index 003255f87966fb753bf5cf1808b2d32c9f4bafbb..7792726006859c58da0b177b040858e0e93bc88f 100644 (file)
@@ -47,14 +47,15 @@ struct CheckAttrVisitor<'a> {
 
 impl<'a> CheckAttrVisitor<'a> {
     /// Check any attribute.
-    fn check_attribute(&self, attr: &ast::Attribute, item: &ast::Item, target: Target) {
-        if let Some(name) = attr.name() {
-            match &*name.as_str() {
-                "inline" => self.check_inline(attr, item, target),
-                "repr" => self.check_repr(attr, item, target),
-                _ => (),
+    fn check_attributes(&self, item: &ast::Item, target: Target) {
+        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.
@@ -66,45 +67,51 @@ fn check_inline(&self, attr: &ast::Attribute, item: &ast::Item, target: Target)
         }
     }
 
-    /// Check if an `#[repr]` attr is valid.
-    fn check_repr(&self, attr: &ast::Attribute, item: &ast::Item, target: Target) {
-        let words = match attr.meta_item_list() {
-            Some(words) => words,
-            None => {
-                return;
-            }
-        };
+    /// Check if the `#[repr]` attributes on `item` are valid.
+    fn check_repr(&self, item: &ast::Item, target: Target) {
+        // Extract the names of all repr hints, e.g., [foo, bar, align] for:
+        // ```
+        // #[repr(foo)]
+        // #[repr(bar, align(8))]
+        // ```
+        let hints: Vec<_> = item.attrs
+            .iter()
+            .filter(|attr| match attr.name() {
+                Some(name) => name == "repr",
+                None => false,
+            })
+            .filter_map(|attr| attr.meta_item_list())
+            .flat_map(|hints| hints)
+            .collect();
 
         let mut int_reprs = 0;
         let mut is_c = false;
         let mut is_simd = false;
 
-        for word in words {
-
-            let name = match word.name() {
-                Some(word) => word,
-                None => continue,
+        for hint in &hints {
+            let name = if let Some(name) = hint.name() {
+                name
+            } else {
+                // Invalid repr hint like repr(42). We don't check for unrecognized hints here
+                // (libsyntax does that), so just ignore it.
+                continue;
             };
 
-            let (message, label) = match &*name.as_str() {
+            let (article, allowed_targets) = match &*name.as_str() {
                 "C" => {
                     is_c = true;
                     if target != Target::Struct &&
                             target != Target::Union &&
                             target != Target::Enum {
-                                ("attribute should be applied to struct, enum or union",
-                                 "a struct, enum or union")
+                                ("a", "struct, enum or union")
                     } else {
                         continue
                     }
                 }
                 "packed" => {
-                    // Do not increment conflicting_reprs here, because "packed"
-                    // can be used to modify another repr hint
                     if target != Target::Struct &&
                             target != Target::Union {
-                                ("attribute should be applied to struct or union",
-                                 "a struct or union")
+                                ("a", "struct or union")
                     } else {
                         continue
                     }
@@ -112,8 +119,7 @@ fn check_repr(&self, attr: &ast::Attribute, item: &ast::Item, target: Target) {
                 "simd" => {
                     is_simd = true;
                     if target != Target::Struct {
-                        ("attribute should be applied to struct",
-                         "a struct")
+                        ("a", "struct")
                     } else {
                         continue
                     }
@@ -121,8 +127,7 @@ fn check_repr(&self, attr: &ast::Attribute, item: &ast::Item, target: Target) {
                 "align" => {
                     if target != Target::Struct &&
                             target != Target::Union {
-                        ("attribute should be applied to struct or union",
-                         "a struct or union")
+                        ("a", "struct or union")
                     } else {
                         continue
                     }
@@ -132,16 +137,16 @@ fn check_repr(&self, attr: &ast::Attribute, item: &ast::Item, target: Target) {
                 "isize" | "usize" => {
                     int_reprs += 1;
                     if target != Target::Enum {
-                        ("attribute should be applied to enum",
-                         "an enum")
+                        ("an", "enum")
                     } else {
                         continue
                     }
                 }
                 _ => continue,
             };
-            struct_span_err!(self.sess, attr.span, E0517, "{}", message)
-                .span_label(item.span, format!("not {}", label))
+            struct_span_err!(self.sess, hint.span, E0517,
+                             "attribute should be applied to {}", allowed_targets)
+                .span_label(item.span, format!("not {} {}", article, allowed_targets))
                 .emit();
         }
 
@@ -149,7 +154,10 @@ fn check_repr(&self, attr: &ast::Attribute, item: &ast::Item, target: Target) {
         if (int_reprs > 1)
            || (is_simd && is_c)
            || (int_reprs == 1 && is_c && is_c_like_enum(item)) {
-            span_warn!(self.sess, attr.span, E0566,
+            // 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,
                        "conflicting representation hints");
         }
     }
@@ -158,9 +166,7 @@ fn check_repr(&self, attr: &ast::Attribute, item: &ast::Item, target: Target) {
 impl<'a> Visitor<'a> for CheckAttrVisitor<'a> {
     fn visit_item(&mut self, item: &'a ast::Item) {
         let target = Target::from_item(item);
-        for attr in &item.attrs {
-            self.check_attribute(attr, item, target);
-        }
+        self.check_attributes(item, target);
         visit::walk_item(self, item);
     }
 }
diff --git a/src/test/compile-fail/attr-usage-repr.rs b/src/test/compile-fail/attr-usage-repr.rs
deleted file mode 100644 (file)
index c0bfd36..0000000
+++ /dev/null
@@ -1,45 +0,0 @@
-// 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 <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.
-
-#![allow(dead_code)]
-#![feature(attr_literals)]
-#![feature(repr_simd)]
-
-#[repr(C)] //~ ERROR: attribute should be applied to struct, enum or union
-fn f() {}
-
-#[repr(C)]
-struct SExtern(f64, f64);
-
-#[repr(packed)]
-struct SPacked(f64, f64);
-
-#[repr(simd)]
-struct SSimd(f64, f64);
-
-#[repr(i8)] //~ ERROR: attribute should be applied to enum
-struct SInt(f64, f64);
-
-#[repr(C)]
-enum EExtern { A, B }
-
-#[repr(align(8))] //~ ERROR: attribute should be applied to struct
-enum EAlign { A, B }
-
-#[repr(packed)] //~ ERROR: attribute should be applied to struct
-enum EPacked { A, B }
-
-#[repr(simd)] //~ ERROR: attribute should be applied to struct
-enum ESimd { A, B }
-
-#[repr(i8)]
-enum EInt { A, B }
-
-fn main() {}
diff --git a/src/test/ui/attr-usage-repr.rs b/src/test/ui/attr-usage-repr.rs
new file mode 100644 (file)
index 0000000..db5cd47
--- /dev/null
@@ -0,0 +1,44 @@
+// 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 <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)]
+#![feature(repr_simd)]
+
+#[repr(C)] //~ ERROR: attribute should be applied to struct, enum or union
+fn f() {}
+
+#[repr(C)]
+struct SExtern(f64, f64);
+
+#[repr(packed)]
+struct SPacked(f64, f64);
+
+#[repr(simd)]
+struct SSimd(f64, f64);
+
+#[repr(i8)] //~ ERROR: attribute should be applied to enum
+struct SInt(f64, f64);
+
+#[repr(C)]
+enum EExtern { A, B }
+
+#[repr(align(8))] //~ ERROR: attribute should be applied to struct
+enum EAlign { A, B }
+
+#[repr(packed)] //~ ERROR: attribute should be applied to struct
+enum EPacked { A, B }
+
+#[repr(simd)] //~ ERROR: attribute should be applied to struct
+enum ESimd { A, B }
+
+#[repr(i8)]
+enum EInt { A, B }
+
+fn main() {}
diff --git a/src/test/ui/attr-usage-repr.stderr b/src/test/ui/attr-usage-repr.stderr
new file mode 100644 (file)
index 0000000..b9c0126
--- /dev/null
@@ -0,0 +1,42 @@
+error[E0517]: attribute should be applied to struct, enum or union
+  --> $DIR/attr-usage-repr.rs:14:8
+   |
+14 | #[repr(C)] //~ ERROR: attribute should be applied to struct, enum or union
+   |        ^
+15 | fn f() {}
+   | --------- not a struct, enum or union
+
+error[E0517]: attribute should be applied to enum
+  --> $DIR/attr-usage-repr.rs:26:8
+   |
+26 | #[repr(i8)] //~ ERROR: attribute should be applied to enum
+   |        ^^
+27 | struct SInt(f64, f64);
+   | ---------------------- not an enum
+
+error[E0517]: attribute should be applied to struct or union
+  --> $DIR/attr-usage-repr.rs:32:8
+   |
+32 | #[repr(align(8))] //~ ERROR: attribute should be applied to struct
+   |        ^^^^^^^^
+33 | enum EAlign { A, B }
+   | -------------------- not a struct or union
+
+error[E0517]: attribute should be applied to struct or union
+  --> $DIR/attr-usage-repr.rs:35:8
+   |
+35 | #[repr(packed)] //~ ERROR: attribute should be applied to struct
+   |        ^^^^^^
+36 | enum EPacked { A, B }
+   | --------------------- not a struct or union
+
+error[E0517]: attribute should be applied to struct
+  --> $DIR/attr-usage-repr.rs:38:8
+   |
+38 | #[repr(simd)] //~ ERROR: attribute should be applied to struct
+   |        ^^^^
+39 | enum ESimd { A, B }
+   | ------------------- not a struct
+
+error: aborting due to 5 previous errors
+
index 31c055f229c3e7a66bdc216563393f6420548ef1..a603658d3161015f4a56551d7c90260ba5f43161 100644 (file)
@@ -13,7 +13,6 @@
 
 #[repr(simd)]
 #[derive(Copy, Clone)]
-#[repr(C)]
 struct LocalSimd(u8, u8);
 
 extern {
index fa47e1a2903ccde8dd4cb005351602eeed5dbb18..ab1ebefa333ea702dcdb72aa74f60d7f7cc17383 100644 (file)
@@ -1,15 +1,15 @@
 error: use of SIMD type `LocalSimd` in FFI is highly experimental and may result in invalid code
-  --> $DIR/feature-gate-simd-ffi.rs:20:17
+  --> $DIR/feature-gate-simd-ffi.rs:19:17
    |
-20 |     fn baz() -> LocalSimd; //~ ERROR use of SIMD type
+19 |     fn baz() -> LocalSimd; //~ ERROR use of SIMD type
    |                 ^^^^^^^^^
    |
    = help: add #![feature(simd_ffi)] to the crate attributes to enable
 
 error: use of SIMD type `LocalSimd` in FFI is highly experimental and may result in invalid code
-  --> $DIR/feature-gate-simd-ffi.rs:21:15
+  --> $DIR/feature-gate-simd-ffi.rs:20:15
    |
-21 |     fn qux(x: LocalSimd); //~ ERROR use of SIMD type
+20 |     fn qux(x: LocalSimd); //~ ERROR use of SIMD type
    |               ^^^^^^^^^
    |
    = help: add #![feature(simd_ffi)] to the crate attributes to enable
diff --git a/src/test/ui/issue-47094.rs b/src/test/ui/issue-47094.rs
new file mode 100644 (file)
index 0000000..3ab9d4e
--- /dev/null
@@ -0,0 +1,26 @@
+// 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.
+
+// must-compile-successfully
+
+#[repr(C,u8)]
+enum Foo {
+    A,
+    B,
+}
+
+#[repr(C)]
+#[repr(u8)]
+enum Bar {
+    A,
+    B,
+}
+
+fn main() {}
diff --git a/src/test/ui/issue-47094.stderr b/src/test/ui/issue-47094.stderr
new file mode 100644 (file)
index 0000000..5276b88
--- /dev/null
@@ -0,0 +1,14 @@
+warning[E0566]: conflicting representation hints
+  --> $DIR/issue-47094.rs:13:8
+   |
+13 | #[repr(C,u8)]
+   |        ^ ^^
+
+warning[E0566]: conflicting representation hints
+  --> $DIR/issue-47094.rs:19:8
+   |
+19 | #[repr(C)]
+   |        ^
+20 | #[repr(u8)]
+   |        ^^
+