Warn about multiple conflicting #[repr] hints

lambda-fairy · lambda-fairy · commit 42b75a5c1846 · 2016-08-31T18:54:19.000+12:00
Closes #34622
diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs
@@ -1804,4 +1804,5 @@ register_diagnostics! {
     E0490, // a value of type `..` is borrowed for too long
     E0491, // in type `..`, reference has a longer lifetime than the data it...
     E0495, // cannot infer an appropriate lifetime due to conflicting requirements
+    E0566  // conflicting representation hints
 }
diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs
@@ -52,6 +52,7 @@ impl<'a> CheckAttrVisitor<'a> {
             }
         };
 
+        let mut conflicting_reprs = 0;
         for word in words {
             let name = match word.name() {
                 Some(word) => word,
@@ -60,13 +61,24 @@ impl<'a> CheckAttrVisitor<'a> {
 
             let message = match &*name {
                 "C" => {
+                    conflicting_reprs += 1;
                     if target != Target::Struct && target != Target::Enum {
                         "attribute should be applied to struct or enum"
                     } else {
                         continue
                     }
                 }
-                "packed" | "simd" => {
+                "packed" => {
+                    // Do not increment conflicting_reprs here, because "packed"
+                    // can be used to modify another repr hint
+                    if target != Target::Struct {
+                        "attribute should be applied to struct"
+                    } else {
+                        continue
+                    }
+                }
+                "simd" => {
+                    conflicting_reprs += 1;
                     if target != Target::Struct {
                         "attribute should be applied to struct"
                     } else {
@@ -76,6 +88,7 @@ impl<'a> CheckAttrVisitor<'a> {
                 "i8" | "u8" | "i16" | "u16" |
                 "i32" | "u32" | "i64" | "u64" |
                 "isize" | "usize" => {
+                    conflicting_reprs += 1;
                     if target != Target::Enum {
                         "attribute should be applied to enum"
                     } else {
@@ -87,6 +100,10 @@ impl<'a> CheckAttrVisitor<'a> {
 
             span_err!(self.sess, attr.span, E0517, "{}", message);
         }
+        if conflicting_reprs > 1 {
+            span_warn!(self.sess, attr.span, E0566,
+                       "conflicting representation hints");
+        }
     }
 
     fn check_attribute(&self, attr: &ast::Attribute, target: Target) {
diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs
@@ -904,9 +904,8 @@ pub fn find_repr_attrs(diagnostic: &Handler, attr: &Attribute) -> Vec<ReprAttr>
                         }
                     };
 
-                    match hint {
-                        Some(h) => acc.push(h),
-                        None => { }
+                    if let Some(h) = hint {
+                        acc.push(h);
                     }
                 } else {
                     span_err!(diagnostic, item.span, E0553,
diff --git a/src/test/compile-fail/conflicting-repr-hints.rs b/src/test/compile-fail/conflicting-repr-hints.rs
@@ -0,0 +1,30 @@
+// Copyright 2016 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(rustc_attrs)]
+#![allow(dead_code)]
+
+#[repr(C)]
+enum A { A }
+
+#[repr(u64)]
+enum B { B }
+
+#[repr(C, u64)] //~ WARNING conflicting representation hints
+enum C { C }
+
+#[repr(u32, u64)] //~ WARNING conflicting representation hints
+enum D { D }
+
+#[repr(C, packed)]
+struct E(i32);
+
+#[rustc_error]
+fn main() {} //~ ERROR compilation successful
diff --git a/src/test/run-pass/mir_adt_construction.rs b/src/test/run-pass/mir_adt_construction.rs
@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#[repr(C, u32)]
+#[repr(C)]
 enum CEnum {
     Hello = 30,
     World = 60

Original file line number	Diff line number	Diff line change
`@@ -1804,4 +1804,5 @@ register_diagnostics! {`
`1804`	`1804`	E0490, // a value of type `..` is borrowed for too long
`1805`	`1805`	E0491, // in type `..`, reference has a longer lifetime than the data it...
`1806`	`1806`	`E0495, // cannot infer an appropriate lifetime due to conflicting requirements`
	`1807`	`+ E0566 // conflicting representation hints`
`1807`	`1808`	`}`
Original file line number	Diff line number	Diff line change
`@@ -904,9 +904,8 @@ pub fn find_repr_attrs(diagnostic: &Handler, attr: &Attribute) -> Vec<ReprAttr>`
`904`	`904`	`}`
`905`	`905`	`};`
`906`	`906`
`907`		`- match hint {`
`908`		`- Some(h) => acc.push(h),`
`909`		`- None => { }`
	`907`	`+ if let Some(h) = hint {`
	`908`	`+ acc.push(h);`
`910`	`909`	`}`
`911`	`910`	`} else {`
`912`	`911`	`span_err!(diagnostic, item.span, E0553,`