Auto merge of #47111 - rkruppe:repr-transparent, r=estebank

Check all repr hints together when checking for mis-applied attributes

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.
This commit is contained in:
bors 2018-01-02 08:34:27 +00:00
commit 5873a74d8f
7 changed files with 130 additions and 44 deletions

View 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 @@ impl<'a> CheckAttrVisitor<'a> {
}
}
/// 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 @@ impl<'a> CheckAttrVisitor<'a> {
"simd" => {
is_simd = true;
if target != Target::Struct {
("attribute should be applied to struct",
"a struct")
("a", "struct")
} else {
continue
}
@ -121,8 +127,7 @@ impl<'a> CheckAttrVisitor<'a> {
"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 @@ impl<'a> CheckAttrVisitor<'a> {
"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 @@ impl<'a> CheckAttrVisitor<'a> {
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 @@ impl<'a> CheckAttrVisitor<'a> {
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);
}
}

View file

@ -8,7 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![allow(dead_code)]
#![feature(attr_literals)]
#![feature(repr_simd)]

View file

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

View file

@ -13,7 +13,6 @@
#[repr(simd)]
#[derive(Copy, Clone)]
#[repr(C)]
struct LocalSimd(u8, u8);
extern {

View 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

View file

@ -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() {}

View file

@ -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)]
| ^^