Skip to content

Commit

Permalink
Auto merge of #77015 - davidtwco:check-attr-variant-closure-expr, r=lcnr
Browse files Browse the repository at this point in the history
passes: `check_attr` on more targets

This PR modifies `check_attr` so that:

- Enum variants are now checked (some attributes would not have been prohibited on variants previously).
- `check_expr_attributes` and `check_stmt_attributes` are removed as `check_attributes` can perform the same checks. This means that codegen attribute errors aren't shown if there are other errors first (e.g. from other attributes, as shown in `src/test/ui/macros/issue-68060.rs` changes below).
  • Loading branch information
bors committed Oct 23, 2020
2 parents 07a63e6 + 57e8fc5 commit 7bade6e
Show file tree
Hide file tree
Showing 20 changed files with 114 additions and 131 deletions.
91 changes: 33 additions & 58 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl CheckAttrVisitor<'tcx> {
return;
}

if matches!(target, Target::Fn | Target::Method(_) | Target::ForeignFn) {
if matches!(target, Target::Closure | Target::Fn | Target::Method(_) | Target::ForeignFn) {
self.tcx.ensure().codegen_fn_attrs(self.tcx.hir().local_def_id(hir_id));
}

Expand Down Expand Up @@ -195,7 +195,7 @@ impl CheckAttrVisitor<'tcx> {
/// Checks if the `#[non_exhaustive]` attribute on an `item` is valid. Returns `true` if valid.
fn check_non_exhaustive(&self, attr: &Attribute, span: &Span, target: Target) -> bool {
match target {
Target::Struct | Target::Enum => true,
Target::Struct | Target::Enum | Target::Variant => true,
_ => {
struct_span_err!(
self.tcx.sess,
Expand Down Expand Up @@ -587,6 +587,9 @@ impl CheckAttrVisitor<'tcx> {

for hint in &hints {
let (article, allowed_targets) = match hint.name_or_empty() {
_ if !matches!(target, Target::Struct | Target::Enum | Target::Union) => {
("a", "struct, enum, or union")
}
name @ sym::C | name @ sym::align => {
is_c |= name == sym::C;
match target {
Expand Down Expand Up @@ -652,12 +655,16 @@ impl CheckAttrVisitor<'tcx> {
}
_ => continue,
};
self.emit_repr_error(

struct_span_err!(
self.tcx.sess,
hint.span(),
*span,
&format!("attribute should be applied to {}", allowed_targets),
&format!("not {} {}", article, allowed_targets),
E0517,
"{}",
&format!("attribute should be applied to {} {}", article, allowed_targets)
)
.span_label(*span, &format!("not {} {}", article, allowed_targets))
.emit();
}

// Just point at all repr hints if there are any incompatibilities.
Expand Down Expand Up @@ -703,56 +710,6 @@ impl CheckAttrVisitor<'tcx> {
}
}

fn emit_repr_error(
&self,
hint_span: Span,
label_span: Span,
hint_message: &str,
label_message: &str,
) {
struct_span_err!(self.tcx.sess, hint_span, E0517, "{}", hint_message)
.span_label(label_span, label_message)
.emit();
}

fn check_stmt_attributes(&self, stmt: &hir::Stmt<'_>) {
// When checking statements ignore expressions, they will be checked later
if let hir::StmtKind::Local(ref l) = stmt.kind {
self.check_attributes(l.hir_id, &l.attrs, &stmt.span, Target::Statement, None);
for attr in l.attrs.iter() {
if self.tcx.sess.check_name(attr, sym::repr) {
self.emit_repr_error(
attr.span,
stmt.span,
"attribute should not be applied to a statement",
"not a struct, enum, or union",
);
}
}
}
}

fn check_expr_attributes(&self, expr: &hir::Expr<'_>) {
let target = match expr.kind {
hir::ExprKind::Closure(..) => Target::Closure,
_ => Target::Expression,
};
self.check_attributes(expr.hir_id, &expr.attrs, &expr.span, target, None);
for attr in expr.attrs.iter() {
if self.tcx.sess.check_name(attr, sym::repr) {
self.emit_repr_error(
attr.span,
expr.span,
"attribute should not be applied to an expression",
"not defining a struct, enum, or union",
);
}
}
if target == Target::Closure {
self.tcx.ensure().codegen_fn_attrs(self.tcx.hir().local_def_id(expr.hir_id));
}
}

fn check_used(&self, attrs: &'hir [Attribute], target: Target) {
for attr in attrs {
if self.tcx.sess.check_name(attr, sym::used) && target != Target::Static {
Expand Down Expand Up @@ -808,14 +765,32 @@ impl Visitor<'tcx> for CheckAttrVisitor<'tcx> {
}

fn visit_stmt(&mut self, stmt: &'tcx hir::Stmt<'tcx>) {
self.check_stmt_attributes(stmt);
// When checking statements ignore expressions, they will be checked later.
if let hir::StmtKind::Local(ref l) = stmt.kind {
self.check_attributes(l.hir_id, &l.attrs, &stmt.span, Target::Statement, None);
}
intravisit::walk_stmt(self, stmt)
}

fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
self.check_expr_attributes(expr);
let target = match expr.kind {
hir::ExprKind::Closure(..) => Target::Closure,
_ => Target::Expression,
};

self.check_attributes(expr.hir_id, &expr.attrs, &expr.span, target, None);
intravisit::walk_expr(self, expr)
}

fn visit_variant(
&mut self,
variant: &'tcx hir::Variant<'tcx>,
generics: &'tcx hir::Generics<'tcx>,
item_id: HirId,
) {
self.check_attributes(variant.id, variant.attrs, &variant.span, Target::Variant, None);
intravisit::walk_variant(self, variant, generics, item_id)
}
}

fn is_c_like_enum(item: &Item<'_>) -> bool {
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/attr-usage-repr.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![feature(repr_simd)]

#[repr(C)] //~ ERROR: attribute should be applied to struct, enum, or union
#[repr(C)] //~ ERROR: attribute should be applied to a struct, enum, or union
fn f() {}

#[repr(C)]
Expand All @@ -12,7 +12,7 @@ struct SPacked(f64, f64);
#[repr(simd)]
struct SSimd(f64, f64);

#[repr(i8)] //~ ERROR: attribute should be applied to enum
#[repr(i8)] //~ ERROR: attribute should be applied to an enum
struct SInt(f64, f64);

#[repr(C)]
Expand All @@ -21,10 +21,10 @@ enum EExtern { A, B }
#[repr(align(8))]
enum EAlign { A, B }

#[repr(packed)] //~ ERROR: attribute should be applied to struct
#[repr(packed)] //~ ERROR: attribute should be applied to a struct
enum EPacked { A, B }

#[repr(simd)] //~ ERROR: attribute should be applied to struct
#[repr(simd)] //~ ERROR: attribute should be applied to a struct
enum ESimd { A, B }

#[repr(i8)]
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/attr-usage-repr.stderr
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
error[E0517]: attribute should be applied to struct, enum, or union
error[E0517]: attribute should be applied to a struct, enum, or union
--> $DIR/attr-usage-repr.rs:3:8
|
LL | #[repr(C)]
| ^
LL | fn f() {}
| --------- not a struct, enum, or union

error[E0517]: attribute should be applied to enum
error[E0517]: attribute should be applied to an enum
--> $DIR/attr-usage-repr.rs:15:8
|
LL | #[repr(i8)]
| ^^
LL | struct SInt(f64, f64);
| ---------------------- not an enum

error[E0517]: attribute should be applied to struct or union
error[E0517]: attribute should be applied to a struct or union
--> $DIR/attr-usage-repr.rs:24:8
|
LL | #[repr(packed)]
| ^^^^^^
LL | enum EPacked { A, B }
| --------------------- not a struct or union

error[E0517]: attribute should be applied to struct
error[E0517]: attribute should be applied to a struct
--> $DIR/attr-usage-repr.rs:27:8
|
LL | #[repr(simd)]
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/error-codes/E0517.stderr
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
error[E0517]: attribute should be applied to struct, enum, or union
error[E0517]: attribute should be applied to a struct, enum, or union
--> $DIR/E0517.rs:1:8
|
LL | #[repr(C)]
| ^
LL | type Foo = u8;
| -------------- not a struct, enum, or union

error[E0517]: attribute should be applied to struct or union
error[E0517]: attribute should be applied to a struct or union
--> $DIR/E0517.rs:4:8
|
LL | #[repr(packed)]
| ^^^^^^
LL | enum Foo2 {Bar, Baz}
| -------------------- not a struct or union

error[E0517]: attribute should be applied to enum
error[E0517]: attribute should be applied to an enum
--> $DIR/E0517.rs:7:8
|
LL | #[repr(u8)]
| ^^
LL | struct Foo3 {bar: bool, baz: bool}
| ---------------------------------- not an enum

error[E0517]: attribute should be applied to struct, enum, or union
error[E0517]: attribute should be applied to a struct, enum, or union
--> $DIR/E0517.rs:10:8
|
LL | #[repr(C)]
Expand Down
7 changes: 7 additions & 0 deletions src/test/ui/inline-disallow-on-variant.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
enum Foo {
#[inline]
//~^ ERROR attribute should be applied
Variant,
}

fn main() {}
12 changes: 12 additions & 0 deletions src/test/ui/inline-disallow-on-variant.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0518]: attribute should be applied to function or closure
--> $DIR/inline-disallow-on-variant.rs:2:5
|
LL | #[inline]
| ^^^^^^^^^
LL |
LL | Variant,
| ------- not a function or closure

error: aborting due to previous error

For more information about this error, try `rustc --explain E0518`.
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-31769.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
fn main() {
#[inline] struct Foo; //~ ERROR attribute should be applied to function or closure
#[repr(C)] fn foo() {} //~ ERROR attribute should be applied to struct, enum, or union
#[repr(C)] fn foo() {} //~ ERROR attribute should be applied to a struct, enum, or union
}
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-31769.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0518]: attribute should be applied to function or closure
LL | #[inline] struct Foo;
| ^^^^^^^^^ ----------- not a function or closure

error[E0517]: attribute should be applied to struct, enum, or union
error[E0517]: attribute should be applied to a struct, enum, or union
--> $DIR/issue-31769.rs:3:12
|
LL | #[repr(C)] fn foo() {}
Expand Down
10 changes: 4 additions & 6 deletions src/test/ui/issues/issue-43988.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,17 @@ fn main() {

#[repr(nothing)]
let _x = 0;
//~^^ ERROR attribute should not be applied to a statement
//~^^ ERROR attribute should be applied to a struct, enum, or union

#[repr(something_not_real)]
loop {
()
};
//~^^^^ ERROR attribute should not be applied to an expression
//~^^^^ ERROR attribute should be applied to a struct, enum, or union

#[repr]
let _y = "123";
//~^^ ERROR attribute should not be applied to a statement
//~| ERROR malformed `repr` attribute
//~^^ ERROR malformed `repr` attribute

fn foo() {}

Expand All @@ -33,6 +32,5 @@ fn main() {
//~^^ ERROR attribute should be applied to function or closure

let _z = #[repr] 1;
//~^ ERROR attribute should not be applied to an expression
//~| ERROR malformed `repr` attribute
//~^ ERROR malformed `repr` attribute
}
34 changes: 10 additions & 24 deletions src/test/ui/issues/issue-43988.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | #[repr]
| ^^^^^^^ help: must be of the form: `#[repr(C)]`

error: malformed `repr` attribute input
--> $DIR/issue-43988.rs:35:14
--> $DIR/issue-43988.rs:34:14
|
LL | let _z = #[repr] 1;
| ^^^^^^^ help: must be of the form: `#[repr(C)]`
Expand All @@ -26,47 +26,33 @@ LL | #[inline(XYZ)]
LL | let _b = 4;
| ----------- not a function or closure

error[E0517]: attribute should not be applied to a statement
--> $DIR/issue-43988.rs:14:5
error[E0517]: attribute should be applied to a struct, enum, or union
--> $DIR/issue-43988.rs:14:12
|
LL | #[repr(nothing)]
| ^^^^^^^^^^^^^^^^
| ^^^^^^^
LL | let _x = 0;
| ----------- not a struct, enum, or union

error[E0517]: attribute should not be applied to an expression
--> $DIR/issue-43988.rs:18:5
error[E0517]: attribute should be applied to a struct, enum, or union
--> $DIR/issue-43988.rs:18:12
|
LL | #[repr(something_not_real)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^
LL | / loop {
LL | | ()
LL | | };
| |_____- not defining a struct, enum, or union

error[E0517]: attribute should not be applied to a statement
--> $DIR/issue-43988.rs:24:5
|
LL | #[repr]
| ^^^^^^^
LL | let _y = "123";
| --------------- not a struct, enum, or union
| |_____- not a struct, enum, or union

error[E0518]: attribute should be applied to function or closure
--> $DIR/issue-43988.rs:31:5
--> $DIR/issue-43988.rs:30:5
|
LL | #[inline(ABC)]
| ^^^^^^^^^^^^^^
LL | foo();
| ----- not a function or closure

error[E0517]: attribute should not be applied to an expression
--> $DIR/issue-43988.rs:35:14
|
LL | let _z = #[repr] 1;
| ^^^^^^^ - not defining a struct, enum, or union

error: aborting due to 9 previous errors
error: aborting due to 7 previous errors

Some errors have detailed explanations: E0517, E0518.
For more information about an error, try `rustc --explain E0517`.
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-74082.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#![allow(dead_code)]

#[repr(i128)] //~ ERROR: attribute should be applied to enum
#[repr(i128)] //~ ERROR: attribute should be applied to an enum
struct Foo;

#[repr(u128)] //~ ERROR: attribute should be applied to enum
#[repr(u128)] //~ ERROR: attribute should be applied to an enum
struct Bar;

fn main() {}
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-74082.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
error[E0517]: attribute should be applied to enum
error[E0517]: attribute should be applied to an enum
--> $DIR/issue-74082.rs:3:8
|
LL | #[repr(i128)]
| ^^^^
LL | struct Foo;
| ----------- not an enum

error[E0517]: attribute should be applied to enum
error[E0517]: attribute should be applied to an enum
--> $DIR/issue-74082.rs:6:8
|
LL | #[repr(u128)]
Expand Down
Loading

0 comments on commit 7bade6e

Please sign in to comment.