From bbe51336c9f710439c8c0e7e794f999538a32729 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 16:56:48 -0500 Subject: [PATCH 01/13] Syntactically permit attributes on 'if' expressions Fixes #68618 Previously, attributes on 'if' expressions (e.g. `#[attr] if true {}`) were disallowed during parsing. This made it impossible for macros to perform any custom handling of such attributes (e.g. stripping them away), since a compilation error would be emitted before they ever had a chance to run. This PR permits attributes on 'if' expressions ('if-attrs' from here on) syntactically, i.e. during parsing. We instead deny if-attrs during AST validation, which occurs after all macro expansions have run. This is a conservative change which allows more code to be processed by macros. It does not commit us to *semantically* accepting if-attrs. For example, the following code is not allowed even with this PR: ```rust fn builtin_attr() { #[allow(warnings)] if true {} } fn custom_attr() { #[my_proc_macro_attr] if true {} } ``` However, the following code *is* accepted ```rust #[cfg(FALSE)] fn foo() { #[allow(warnings)] if true {} #[my_custom_attr] if true {} } #[my_custom_attr] fn use_within_body() { #[allow(warnings)] if true {} #[my_custom_attr] if true {} } ``` --- src/librustc_ast_passes/ast_validation.rs | 10 ++++++++++ src/librustc_parse/parser/expr.rs | 8 -------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 152086bfce0ea..0f6d73562f492 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -399,6 +399,15 @@ impl<'a> AstValidator<'a> { } } } + + fn check_attr_on_if_expr(&self, attrs: &[Attribute]) { + if let [a0, ..] = attrs { + // Just point to the first attribute in there... + self.err_handler() + .struct_span_err(a0.span, "attributes are not yet allowed on `if` expressions") + .emit(); + } + } } enum GenericPosition { @@ -515,6 +524,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { ) .emit(); } + ExprKind::If(..) => self.check_attr_on_if_expr(&*expr.attrs), _ => {} } diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs index 098c8355ab944..3ff855abb3d15 100644 --- a/src/librustc_parse/parser/expr.rs +++ b/src/librustc_parse/parser/expr.rs @@ -676,19 +676,11 @@ impl<'a> Parser<'a> { expr.map(|mut expr| { attrs.extend::>(expr.attrs.into()); expr.attrs = attrs; - self.error_attr_on_if_expr(&expr); expr }) }) } - fn error_attr_on_if_expr(&self, expr: &Expr) { - if let (ExprKind::If(..), [a0, ..]) = (&expr.kind, &*expr.attrs) { - // Just point to the first attribute in there... - self.struct_span_err(a0.span, "attributes are not yet allowed on `if` expressions") - .emit(); - } - } fn parse_dot_or_call_expr_with_(&mut self, mut e: P, lo: Span) -> PResult<'a, P> { loop { From 422ae4cb5de316fd014071684c8537c657230cb7 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 17:17:43 -0500 Subject: [PATCH 02/13] Add some tests --- .../ui/parser/if-attrs/cfg-false-if-attr.rs | 31 +++++++++++++++++++ .../if-attrs/deny-if-attr-validation.rs | 9 ++++++ .../if-attrs/deny-if-attr-validation.stderr | 14 +++++++++ .../ui/parser/if-attrs/let-chains-attr.rs | 13 ++++++++ .../ui/parser/if-attrs/let-chains-attr.stderr | 8 +++++ 5 files changed, 75 insertions(+) create mode 100644 src/test/ui/parser/if-attrs/cfg-false-if-attr.rs create mode 100644 src/test/ui/parser/if-attrs/deny-if-attr-validation.rs create mode 100644 src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr create mode 100644 src/test/ui/parser/if-attrs/let-chains-attr.rs create mode 100644 src/test/ui/parser/if-attrs/let-chains-attr.stderr diff --git a/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs b/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs new file mode 100644 index 0000000000000..2932ec1a23195 --- /dev/null +++ b/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs @@ -0,0 +1,31 @@ +// check-pass + +#[cfg(FALSE)] +fn simple_attr() { + #[attr] if true {} + #[allow_warnings] if true {} +} + +#[cfg(FALSE)] +fn if_else_chain() { + #[first_attr] if true { + } else if false { + } else { + } +} + +#[cfg(FALSE)] +fn if_let() { + #[attr] if let Some(_) = Some(true) {} +} + +macro_rules! custom_macro { + ($expr:expr) => {} +} + +custom_macro! { + #[attr] if true {} +} + + +fn main() {} diff --git a/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs b/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs new file mode 100644 index 0000000000000..f2b7fd2dc53c1 --- /dev/null +++ b/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs @@ -0,0 +1,9 @@ +fn main() { + #[allow(warnings)] if true {} //~ ERROR attributes are not yet allowed on `if` expressions + if false { + } else if true { + } + + #[allow(warnings)] if let Some(_) = Some(true) { //~ ERROR attributes are not yet allowed on `if` expressions + } +} diff --git a/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr b/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr new file mode 100644 index 0000000000000..b4c9b9755976c --- /dev/null +++ b/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr @@ -0,0 +1,14 @@ +error: attributes are not yet allowed on `if` expressions + --> $DIR/deny-if-attr-validation.rs:2:5 + | +LL | #[allow(warnings)] if true {} + | ^^^^^^^^^^^^^^^^^^ + +error: attributes are not yet allowed on `if` expressions + --> $DIR/deny-if-attr-validation.rs:7:2 + | +LL | #[allow(warnings)] if let Some(_) = Some(true) { + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/parser/if-attrs/let-chains-attr.rs b/src/test/ui/parser/if-attrs/let-chains-attr.rs new file mode 100644 index 0000000000000..3275f6f89e538 --- /dev/null +++ b/src/test/ui/parser/if-attrs/let-chains-attr.rs @@ -0,0 +1,13 @@ +// check-pass + +#![feature(let_chains)] //~ WARN the feature `let_chains` is incomplete + +#[cfg(FALSE)] +fn foo() { + #[attr] + if let Some(_) = Some(true) && let Ok(_) = Ok(1) { + } else if let Some(false) = Some(true) { + } +} + +fn main() {} diff --git a/src/test/ui/parser/if-attrs/let-chains-attr.stderr b/src/test/ui/parser/if-attrs/let-chains-attr.stderr new file mode 100644 index 0000000000000..a6c91bb9203b3 --- /dev/null +++ b/src/test/ui/parser/if-attrs/let-chains-attr.stderr @@ -0,0 +1,8 @@ +warning: the feature `let_chains` is incomplete and may cause the compiler to crash + --> $DIR/let-chains-attr.rs:3:12 + | +LL | #![feature(let_chains)] + | ^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + From 0103e57ac06be692a2b0fb7fdf1bf99e38a1e4c7 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 17:20:39 -0500 Subject: [PATCH 03/13] Test that attrs on 'else' and 'else if' don't parse We may want to change this, but it would be a much larger change than this PR is making. --- src/test/ui/parser/if-attrs/else-attrs.rs | 25 +++++++++++++++++ src/test/ui/parser/if-attrs/else-attrs.stderr | 27 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 src/test/ui/parser/if-attrs/else-attrs.rs create mode 100644 src/test/ui/parser/if-attrs/else-attrs.stderr diff --git a/src/test/ui/parser/if-attrs/else-attrs.rs b/src/test/ui/parser/if-attrs/else-attrs.rs new file mode 100644 index 0000000000000..4394b2100c1b5 --- /dev/null +++ b/src/test/ui/parser/if-attrs/else-attrs.rs @@ -0,0 +1,25 @@ +#[cfg(FALSE)] +fn if_else_parse_error() { + if true { + } #[attr] else if false { //~ ERROR expected + } +} + +#[cfg(FALSE)] +fn else_attr_ifparse_error() { + if true { + } else #[attr] if false { //~ ERROR expected + } else { + } +} + +#[cfg(FALSE)] +fn else_parse_error() { + if true { + } else if false { + } #[attr] else { //~ ERROR expected + } +} + +fn main() { +} diff --git a/src/test/ui/parser/if-attrs/else-attrs.stderr b/src/test/ui/parser/if-attrs/else-attrs.stderr new file mode 100644 index 0000000000000..af25b6abc0a3a --- /dev/null +++ b/src/test/ui/parser/if-attrs/else-attrs.stderr @@ -0,0 +1,27 @@ +error: expected expression, found keyword `else` + --> $DIR/else-attrs.rs:4:15 + | +LL | } #[attr] else if false { + | ^^^^ expected expression + +error: expected `{`, found `#` + --> $DIR/else-attrs.rs:11:12 + | +LL | } else #[attr] if false { + | ^ expected `{` + | +help: try placing this code inside a block + | +LL | } else #[attr] { if false { +LL | } else { +LL | } } + | + +error: expected expression, found keyword `else` + --> $DIR/else-attrs.rs:20:15 + | +LL | } #[attr] else { + | ^^^^ expected expression + +error: aborting due to 3 previous errors + From 5c2d6f57b251884622d5f9e9b462d5850f07ebc3 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 17:24:58 -0500 Subject: [PATCH 04/13] Add pretty-print test --- src/test/pretty/if-attr.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 src/test/pretty/if-attr.rs diff --git a/src/test/pretty/if-attr.rs b/src/test/pretty/if-attr.rs new file mode 100644 index 0000000000000..9dcc8ba40b842 --- /dev/null +++ b/src/test/pretty/if-attr.rs @@ -0,0 +1,10 @@ +// pp-exact + +#[cfg(FALSE)] +fn bar() { + + #[attr] + if true { } +} + +fn main() { } From 2768365531ff4127801b101776b5bb89eeeb44b6 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 17:25:33 -0500 Subject: [PATCH 05/13] Run fmt --- src/librustc_parse/parser/expr.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs index 3ff855abb3d15..073f065b4b66a 100644 --- a/src/librustc_parse/parser/expr.rs +++ b/src/librustc_parse/parser/expr.rs @@ -681,7 +681,6 @@ impl<'a> Parser<'a> { }) } - fn parse_dot_or_call_expr_with_(&mut self, mut e: P, lo: Span) -> PResult<'a, P> { loop { if self.eat(&token::Question) { From 1f5776ed9a78df9b8eb5a0129d9c40601fe9eab9 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 17:27:26 -0500 Subject: [PATCH 06/13] Tidy fixes --- src/test/ui/parser/if-attrs/deny-if-attr-validation.rs | 4 ++-- src/test/ui/parser/if-attrs/let-chains-attr.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs b/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs index f2b7fd2dc53c1..361fddd4ca3a3 100644 --- a/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs +++ b/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs @@ -4,6 +4,6 @@ fn main() { } else if true { } - #[allow(warnings)] if let Some(_) = Some(true) { //~ ERROR attributes are not yet allowed on `if` expressions - } + #[allow(warnings)] if let Some(_) = Some(true) { //~ ERROR attributes are not yet allowed + } } diff --git a/src/test/ui/parser/if-attrs/let-chains-attr.rs b/src/test/ui/parser/if-attrs/let-chains-attr.rs index 3275f6f89e538..5237a9ff3961a 100644 --- a/src/test/ui/parser/if-attrs/let-chains-attr.rs +++ b/src/test/ui/parser/if-attrs/let-chains-attr.rs @@ -4,10 +4,10 @@ #[cfg(FALSE)] fn foo() { - #[attr] + #[attr] if let Some(_) = Some(true) && let Ok(_) = Ok(1) { } else if let Some(false) = Some(true) { - } + } } fn main() {} From f67d6db8799f75de41dcdf8b989e8d414485cfb1 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 17:40:24 -0500 Subject: [PATCH 07/13] Update existing tests --- src/test/ui/parser/attr-stmt-expr-attr-bad.rs | 10 +- .../ui/parser/attr-stmt-expr-attr-bad.stderr | 94 +++++++------------ .../if-attrs/deny-if-attr-validation.stderr | 2 +- 3 files changed, 38 insertions(+), 68 deletions(-) diff --git a/src/test/ui/parser/attr-stmt-expr-attr-bad.rs b/src/test/ui/parser/attr-stmt-expr-attr-bad.rs index 118bff8144c7f..f3980a596481c 100644 --- a/src/test/ui/parser/attr-stmt-expr-attr-bad.rs +++ b/src/test/ui/parser/attr-stmt-expr-attr-bad.rs @@ -38,8 +38,6 @@ fn main() {} //~^ ERROR an inner attribute is not permitted in this context #[cfg(FALSE)] fn e() { let _ = #[attr] &mut #![attr] 0; } //~^ ERROR an inner attribute is not permitted in this context -#[cfg(FALSE)] fn e() { let _ = #[attr] if 0 {}; } -//~^ ERROR attributes are not yet allowed on `if` expressions #[cfg(FALSE)] fn e() { let _ = if 0 #[attr] {}; } //~^ ERROR expected `{`, found `#` #[cfg(FALSE)] fn e() { let _ = if 0 {#![attr]}; } @@ -51,14 +49,11 @@ fn main() {} #[cfg(FALSE)] fn e() { let _ = if 0 {} else {#![attr]}; } //~^ ERROR an inner attribute is not permitted in this context #[cfg(FALSE)] fn e() { let _ = if 0 {} else #[attr] if 0 {}; } -//~^ ERROR attributes are not yet allowed on `if` expressions -//~| ERROR expected `{`, found `#` +//~^ ERROR expected `{`, found `#` #[cfg(FALSE)] fn e() { let _ = if 0 {} else if 0 #[attr] {}; } //~^ ERROR expected `{`, found `#` #[cfg(FALSE)] fn e() { let _ = if 0 {} else if 0 {#![attr]}; } //~^ ERROR an inner attribute is not permitted in this context -#[cfg(FALSE)] fn e() { let _ = #[attr] if let _ = 0 {}; } -//~^ ERROR attributes are not yet allowed on `if` expressions #[cfg(FALSE)] fn e() { let _ = if let _ = 0 #[attr] {}; } //~^ ERROR expected `{`, found `#` #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {#![attr]}; } @@ -70,8 +65,7 @@ fn main() {} #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else {#![attr]}; } //~^ ERROR an inner attribute is not permitted in this context #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else #[attr] if let _ = 0 {}; } -//~^ ERROR attributes are not yet allowed on `if` expressions -//~| ERROR expected `{`, found `#` +//~^ ERROR expected `{`, found `#` #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else if let _ = 0 #[attr] {}; } //~^ ERROR expected `{`, found `#` #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else if let _ = 0 {#![attr]}; } diff --git a/src/test/ui/parser/attr-stmt-expr-attr-bad.stderr b/src/test/ui/parser/attr-stmt-expr-attr-bad.stderr index 4775b9b7bc003..98a287e6cd9c1 100644 --- a/src/test/ui/parser/attr-stmt-expr-attr-bad.stderr +++ b/src/test/ui/parser/attr-stmt-expr-attr-bad.stderr @@ -136,14 +136,8 @@ LL | #[cfg(FALSE)] fn e() { let _ = #[attr] &mut #![attr] 0; } | = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. -error: attributes are not yet allowed on `if` expressions - --> $DIR/attr-stmt-expr-attr-bad.rs:41:32 - | -LL | #[cfg(FALSE)] fn e() { let _ = #[attr] if 0 {}; } - | ^^^^^^^ - error: expected `{`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:43:37 + --> $DIR/attr-stmt-expr-attr-bad.rs:41:37 | LL | #[cfg(FALSE)] fn e() { let _ = if 0 #[attr] {}; } | -- ^ --- help: try placing this code inside a block: `{ {}; }` @@ -152,7 +146,7 @@ LL | #[cfg(FALSE)] fn e() { let _ = if 0 #[attr] {}; } | this `if` expression has a condition, but no block error: an inner attribute is not permitted in this context - --> $DIR/attr-stmt-expr-attr-bad.rs:45:38 + --> $DIR/attr-stmt-expr-attr-bad.rs:43:38 | LL | #[cfg(FALSE)] fn e() { let _ = if 0 {#![attr]}; } | ^^^^^^^^ @@ -160,13 +154,13 @@ LL | #[cfg(FALSE)] fn e() { let _ = if 0 {#![attr]}; } = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. error: expected one of `.`, `;`, `?`, `else`, or an operator, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:47:40 + --> $DIR/attr-stmt-expr-attr-bad.rs:45:40 | LL | #[cfg(FALSE)] fn e() { let _ = if 0 {} #[attr] else {}; } | ^ expected one of `.`, `;`, `?`, `else`, or an operator error: expected `{`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:49:45 + --> $DIR/attr-stmt-expr-attr-bad.rs:47:45 | LL | #[cfg(FALSE)] fn e() { let _ = if 0 {} else #[attr] {}; } | ^ --- help: try placing this code inside a block: `{ {}; }` @@ -174,21 +168,15 @@ LL | #[cfg(FALSE)] fn e() { let _ = if 0 {} else #[attr] {}; } | expected `{` error: an inner attribute is not permitted in this context - --> $DIR/attr-stmt-expr-attr-bad.rs:51:46 + --> $DIR/attr-stmt-expr-attr-bad.rs:49:46 | LL | #[cfg(FALSE)] fn e() { let _ = if 0 {} else {#![attr]}; } | ^^^^^^^^ | = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. -error: attributes are not yet allowed on `if` expressions - --> $DIR/attr-stmt-expr-attr-bad.rs:53:45 - | -LL | #[cfg(FALSE)] fn e() { let _ = if 0 {} else #[attr] if 0 {}; } - | ^^^^^^^ - error: expected `{`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:53:45 + --> $DIR/attr-stmt-expr-attr-bad.rs:51:45 | LL | #[cfg(FALSE)] fn e() { let _ = if 0 {} else #[attr] if 0 {}; } | ^ -------- help: try placing this code inside a block: `{ if 0 {}; }` @@ -196,7 +184,7 @@ LL | #[cfg(FALSE)] fn e() { let _ = if 0 {} else #[attr] if 0 {}; } | expected `{` error: expected `{`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:56:50 + --> $DIR/attr-stmt-expr-attr-bad.rs:53:50 | LL | #[cfg(FALSE)] fn e() { let _ = if 0 {} else if 0 #[attr] {}; } | -- ^ --- help: try placing this code inside a block: `{ {}; }` @@ -205,21 +193,15 @@ LL | #[cfg(FALSE)] fn e() { let _ = if 0 {} else if 0 #[attr] {}; } | this `if` expression has a condition, but no block error: an inner attribute is not permitted in this context - --> $DIR/attr-stmt-expr-attr-bad.rs:58:51 + --> $DIR/attr-stmt-expr-attr-bad.rs:55:51 | LL | #[cfg(FALSE)] fn e() { let _ = if 0 {} else if 0 {#![attr]}; } | ^^^^^^^^ | = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. -error: attributes are not yet allowed on `if` expressions - --> $DIR/attr-stmt-expr-attr-bad.rs:60:32 - | -LL | #[cfg(FALSE)] fn e() { let _ = #[attr] if let _ = 0 {}; } - | ^^^^^^^ - error: expected `{`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:62:45 + --> $DIR/attr-stmt-expr-attr-bad.rs:57:45 | LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 #[attr] {}; } | -- ^ --- help: try placing this code inside a block: `{ {}; }` @@ -228,7 +210,7 @@ LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 #[attr] {}; } | this `if` expression has a condition, but no block error: an inner attribute is not permitted in this context - --> $DIR/attr-stmt-expr-attr-bad.rs:64:46 + --> $DIR/attr-stmt-expr-attr-bad.rs:59:46 | LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {#![attr]}; } | ^^^^^^^^ @@ -236,13 +218,13 @@ LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {#![attr]}; } = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. error: expected one of `.`, `;`, `?`, `else`, or an operator, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:66:48 + --> $DIR/attr-stmt-expr-attr-bad.rs:61:48 | LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} #[attr] else {}; } | ^ expected one of `.`, `;`, `?`, `else`, or an operator error: expected `{`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:68:53 + --> $DIR/attr-stmt-expr-attr-bad.rs:63:53 | LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else #[attr] {}; } | ^ --- help: try placing this code inside a block: `{ {}; }` @@ -250,21 +232,15 @@ LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else #[attr] {}; } | expected `{` error: an inner attribute is not permitted in this context - --> $DIR/attr-stmt-expr-attr-bad.rs:70:54 + --> $DIR/attr-stmt-expr-attr-bad.rs:65:54 | LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else {#![attr]}; } | ^^^^^^^^ | = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. -error: attributes are not yet allowed on `if` expressions - --> $DIR/attr-stmt-expr-attr-bad.rs:72:53 - | -LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else #[attr] if let _ = 0 {}; } - | ^^^^^^^ - error: expected `{`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:72:53 + --> $DIR/attr-stmt-expr-attr-bad.rs:67:53 | LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else #[attr] if let _ = 0 {}; } | ^ ---------------- help: try placing this code inside a block: `{ if let _ = 0 {}; }` @@ -272,7 +248,7 @@ LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else #[attr] if let _ = 0 {} | expected `{` error: expected `{`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:75:66 + --> $DIR/attr-stmt-expr-attr-bad.rs:69:66 | LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else if let _ = 0 #[attr] {}; } | -- ^ --- help: try placing this code inside a block: `{ {}; }` @@ -281,7 +257,7 @@ LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else if let _ = 0 #[attr] {} | this `if` expression has a condition, but no block error: an inner attribute is not permitted in this context - --> $DIR/attr-stmt-expr-attr-bad.rs:77:67 + --> $DIR/attr-stmt-expr-attr-bad.rs:71:67 | LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else if let _ = 0 {#![attr]}; } | ^^^^^^^^ @@ -289,7 +265,7 @@ LL | #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else if let _ = 0 {#![attr]} = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. error: an inner attribute is not permitted following an outer attribute - --> $DIR/attr-stmt-expr-attr-bad.rs:80:32 + --> $DIR/attr-stmt-expr-attr-bad.rs:74:32 | LL | #[cfg(FALSE)] fn s() { #[attr] #![attr] let _ = 0; } | ------- ^^^^^^^^ not permitted following an outer attibute @@ -299,7 +275,7 @@ LL | #[cfg(FALSE)] fn s() { #[attr] #![attr] let _ = 0; } = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. error: an inner attribute is not permitted following an outer attribute - --> $DIR/attr-stmt-expr-attr-bad.rs:82:32 + --> $DIR/attr-stmt-expr-attr-bad.rs:76:32 | LL | #[cfg(FALSE)] fn s() { #[attr] #![attr] 0; } | ------- ^^^^^^^^ not permitted following an outer attibute @@ -309,7 +285,7 @@ LL | #[cfg(FALSE)] fn s() { #[attr] #![attr] 0; } = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. error: an inner attribute is not permitted following an outer attribute - --> $DIR/attr-stmt-expr-attr-bad.rs:84:32 + --> $DIR/attr-stmt-expr-attr-bad.rs:78:32 | LL | #[cfg(FALSE)] fn s() { #[attr] #![attr] foo!(); } | ------- ^^^^^^^^ not permitted following an outer attibute @@ -319,7 +295,7 @@ LL | #[cfg(FALSE)] fn s() { #[attr] #![attr] foo!(); } = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. error: an inner attribute is not permitted following an outer attribute - --> $DIR/attr-stmt-expr-attr-bad.rs:86:32 + --> $DIR/attr-stmt-expr-attr-bad.rs:80:32 | LL | #[cfg(FALSE)] fn s() { #[attr] #![attr] foo![]; } | ------- ^^^^^^^^ not permitted following an outer attibute @@ -329,7 +305,7 @@ LL | #[cfg(FALSE)] fn s() { #[attr] #![attr] foo![]; } = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. error: an inner attribute is not permitted following an outer attribute - --> $DIR/attr-stmt-expr-attr-bad.rs:88:32 + --> $DIR/attr-stmt-expr-attr-bad.rs:82:32 | LL | #[cfg(FALSE)] fn s() { #[attr] #![attr] foo!{}; } | ------- ^^^^^^^^ not permitted following an outer attibute @@ -339,7 +315,7 @@ LL | #[cfg(FALSE)] fn s() { #[attr] #![attr] foo!{}; } = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them. error[E0586]: inclusive range with no end - --> $DIR/attr-stmt-expr-attr-bad.rs:94:35 + --> $DIR/attr-stmt-expr-attr-bad.rs:88:35 | LL | #[cfg(FALSE)] fn e() { match 0 { 0..=#[attr] 10 => () } } | ^^^ help: use `..` instead @@ -347,13 +323,13 @@ LL | #[cfg(FALSE)] fn e() { match 0 { 0..=#[attr] 10 => () } } = note: inclusive ranges must be bounded at the end (`..=b` or `a..=b`) error: expected one of `=>`, `if`, or `|`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:94:38 + --> $DIR/attr-stmt-expr-attr-bad.rs:88:38 | LL | #[cfg(FALSE)] fn e() { match 0 { 0..=#[attr] 10 => () } } | ^ expected one of `=>`, `if`, or `|` error[E0586]: inclusive range with no end - --> $DIR/attr-stmt-expr-attr-bad.rs:97:35 + --> $DIR/attr-stmt-expr-attr-bad.rs:91:35 | LL | #[cfg(FALSE)] fn e() { match 0 { 0..=#[attr] -10 => () } } | ^^^ help: use `..` instead @@ -361,19 +337,19 @@ LL | #[cfg(FALSE)] fn e() { match 0 { 0..=#[attr] -10 => () } } = note: inclusive ranges must be bounded at the end (`..=b` or `a..=b`) error: expected one of `=>`, `if`, or `|`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:97:38 + --> $DIR/attr-stmt-expr-attr-bad.rs:91:38 | LL | #[cfg(FALSE)] fn e() { match 0 { 0..=#[attr] -10 => () } } | ^ expected one of `=>`, `if`, or `|` error: unexpected token: `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:100:39 + --> $DIR/attr-stmt-expr-attr-bad.rs:94:39 | LL | #[cfg(FALSE)] fn e() { match 0 { 0..=-#[attr] 10 => () } } | ^ error[E0586]: inclusive range with no end - --> $DIR/attr-stmt-expr-attr-bad.rs:102:35 + --> $DIR/attr-stmt-expr-attr-bad.rs:96:35 | LL | #[cfg(FALSE)] fn e() { match 0 { 0..=#[attr] FOO => () } } | ^^^ help: use `..` instead @@ -381,47 +357,47 @@ LL | #[cfg(FALSE)] fn e() { match 0 { 0..=#[attr] FOO => () } } = note: inclusive ranges must be bounded at the end (`..=b` or `a..=b`) error: expected one of `=>`, `if`, or `|`, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:102:38 + --> $DIR/attr-stmt-expr-attr-bad.rs:96:38 | LL | #[cfg(FALSE)] fn e() { match 0 { 0..=#[attr] FOO => () } } | ^ expected one of `=>`, `if`, or `|` error: unexpected token: `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:106:34 + --> $DIR/attr-stmt-expr-attr-bad.rs:100:34 | LL | #[cfg(FALSE)] fn e() { let _ = x.#![attr]foo(); } | ^ error: expected one of `.`, `;`, `?`, or an operator, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:106:34 + --> $DIR/attr-stmt-expr-attr-bad.rs:100:34 | LL | #[cfg(FALSE)] fn e() { let _ = x.#![attr]foo(); } | ^ expected one of `.`, `;`, `?`, or an operator error: unexpected token: `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:109:34 + --> $DIR/attr-stmt-expr-attr-bad.rs:103:34 | LL | #[cfg(FALSE)] fn e() { let _ = x.#[attr]foo(); } | ^ error: expected one of `.`, `;`, `?`, or an operator, found `#` - --> $DIR/attr-stmt-expr-attr-bad.rs:109:34 + --> $DIR/attr-stmt-expr-attr-bad.rs:103:34 | LL | #[cfg(FALSE)] fn e() { let _ = x.#[attr]foo(); } | ^ expected one of `.`, `;`, `?`, or an operator error: expected statement after outer attribute - --> $DIR/attr-stmt-expr-attr-bad.rs:114:44 + --> $DIR/attr-stmt-expr-attr-bad.rs:108:44 | LL | #[cfg(FALSE)] fn e() { { fn foo() { #[attr]; } } } | ^ error: expected statement after outer attribute - --> $DIR/attr-stmt-expr-attr-bad.rs:116:45 + --> $DIR/attr-stmt-expr-attr-bad.rs:110:45 | LL | #[cfg(FALSE)] fn e() { { fn foo() { #[attr] } } } | ^ -error: aborting due to 57 previous errors +error: aborting due to 53 previous errors For more information about this error, try `rustc --explain E0586`. diff --git a/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr b/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr index b4c9b9755976c..3326d05dbda1c 100644 --- a/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr +++ b/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr @@ -5,7 +5,7 @@ LL | #[allow(warnings)] if true {} | ^^^^^^^^^^^^^^^^^^ error: attributes are not yet allowed on `if` expressions - --> $DIR/deny-if-attr-validation.rs:7:2 + --> $DIR/deny-if-attr-validation.rs:7:5 | LL | #[allow(warnings)] if let Some(_) = Some(true) { | ^^^^^^^^^^^^^^^^^^ From 4aaa2a595ea5ae79d9ee32ba805c7248410d30b7 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 19:06:38 -0500 Subject: [PATCH 08/13] Expand pretty-print test --- src/test/pretty/if-attr.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/test/pretty/if-attr.rs b/src/test/pretty/if-attr.rs index 9dcc8ba40b842..463476737a92f 100644 --- a/src/test/pretty/if-attr.rs +++ b/src/test/pretty/if-attr.rs @@ -1,10 +1,28 @@ // pp-exact #[cfg(FALSE)] -fn bar() { +fn simple_attr() { #[attr] if true { } + + #[allow_warnings] + if true { } +} + +#[cfg(FALSE)] +fn if_else_chain() { + + #[first_attr] + if true { } else if false { } else { } } +#[cfg(FALSE)] +fn if_let() { + + #[attr] + if let Some(_) = Some(true) { } +} + + fn main() { } From f5d40c49ad0b2659a3ccb77e59a63bde365c65e0 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 19:09:00 -0500 Subject: [PATCH 09/13] Add another test --- src/test/ui/parser/if-attrs/cfg-false-if-attr.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs b/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs index 2932ec1a23195..ca4c5cbbaf3ac 100644 --- a/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs +++ b/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs @@ -19,6 +19,11 @@ fn if_let() { #[attr] if let Some(_) = Some(true) {} } +fn bar() {} +fn foo() { + bar(#[cfg(FALSE)] if true {}); +} + macro_rules! custom_macro { ($expr:expr) => {} } From 2c02693d1319b327a3a03b1ab7c3ecca0b346213 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 21:07:41 -0500 Subject: [PATCH 10/13] Deny #[cfg] and #[cfg_attr] on 'if' expressions These are evaluated prior to AST validation, so we need to check for them separately. --- src/librustc_ast_passes/ast_validation.rs | 11 +--- src/librustc_parse/config.rs | 1 + src/libsyntax/attr/mod.rs | 57 ++++++++++++++++++- .../if-attrs/deny-if-attr-validation.rs | 7 +++ .../if-attrs/deny-if-attr-validation.stderr | 4 +- 5 files changed, 66 insertions(+), 14 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 0f6d73562f492..0dd2111178fec 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -399,15 +399,6 @@ impl<'a> AstValidator<'a> { } } } - - fn check_attr_on_if_expr(&self, attrs: &[Attribute]) { - if let [a0, ..] = attrs { - // Just point to the first attribute in there... - self.err_handler() - .struct_span_err(a0.span, "attributes are not yet allowed on `if` expressions") - .emit(); - } - } } enum GenericPosition { @@ -524,7 +515,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { ) .emit(); } - ExprKind::If(..) => self.check_attr_on_if_expr(&*expr.attrs), + ExprKind::If(..) => attr::check_attr_on_if_expr(&*expr.attrs, self.err_handler()), _ => {} } diff --git a/src/librustc_parse/config.rs b/src/librustc_parse/config.rs index bf696faf2f3f4..490fdbc197dd3 100644 --- a/src/librustc_parse/config.rs +++ b/src/librustc_parse/config.rs @@ -251,6 +251,7 @@ const CFG_ATTR_NOTE_REF: &str = "for more information, visit \ impl<'a> StripUnconfigured<'a> { pub fn configure(&mut self, mut node: T) -> Option { + node.check_cfg_attrs(&self.sess.span_diagnostic); self.process_cfg_attrs(&mut node); self.in_cfg(node.attrs()).then_some(node) } diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index ec05dab451af8..1bca95fcdc814 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -10,7 +10,7 @@ pub use StabilityLevel::*; use crate::ast; use crate::ast::{AttrId, AttrItem, AttrKind, AttrStyle, AttrVec, Ident, Name, Path, PathSegment}; -use crate::ast::{Expr, GenericParam, Item, Lit, LitKind, Local, Stmt, StmtKind}; +use crate::ast::{Expr, ExprKind, GenericParam, Item, Lit, LitKind, Local, Stmt, StmtKind}; use crate::ast::{MacArgs, MacDelimiter, MetaItem, MetaItemKind, NestedMetaItem}; use crate::mut_visit::visit_clobber; use crate::ptr::P; @@ -20,12 +20,25 @@ use crate::GLOBALS; use rustc_span::source_map::{BytePos, Spanned}; use rustc_span::symbol::{sym, Symbol}; +use rustc_errors::Handler; use rustc_span::Span; use log::debug; use std::iter; use std::ops::DerefMut; +/// The central location for denying the precense of attributes on an 'if' expression. +/// If attributes on 'if' expres are ever permitted, this is the place to add +/// the feature-gate check. +pub fn check_attr_on_if_expr(attrs: &[Attribute], err_handler: &Handler) { + if let [a0, ..] = attrs { + // Just point to the first attribute in there... + err_handler + .struct_span_err(a0.span, "attributes are not yet allowed on `if` expressions") + .emit(); + } +} + pub fn mark_used(attr: &Attribute) { debug!("marking {:?} as used", attr); GLOBALS.with(|globals| { @@ -626,11 +639,19 @@ impl NestedMetaItem { } pub trait HasAttrs: Sized { + /// A hook invoked before any `#[cfg]` or `#[cfg_attr]` + /// attributes are processed. Override this if you want + /// to prevent `#[cfg]` or `#[cfg_attr]` from being used + /// on something (e.g. an `Expr`) + fn check_cfg_attrs(&self, _handler: &Handler) {} fn attrs(&self) -> &[ast::Attribute]; fn visit_attrs)>(&mut self, f: F); } impl HasAttrs for Spanned { + fn check_cfg_attrs(&self, handler: &Handler) { + self.node.check_cfg_attrs(handler) + } fn attrs(&self) -> &[ast::Attribute] { self.node.attrs() } @@ -662,6 +683,9 @@ impl HasAttrs for AttrVec { } impl HasAttrs for P { + fn check_cfg_attrs(&self, handler: &Handler) { + (**self).check_cfg_attrs(handler); + } fn attrs(&self) -> &[Attribute] { (**self).attrs() } @@ -671,6 +695,16 @@ impl HasAttrs for P { } impl HasAttrs for StmtKind { + fn check_cfg_attrs(&self, handler: &Handler) { + debug!("check_cfg_attrs: StmtKind {:?}", self); + match *self { + StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => { + expr.check_cfg_attrs(handler); + }, + _ => {} + } + } + fn attrs(&self) -> &[Attribute] { match *self { StmtKind::Local(ref local) => local.attrs(), @@ -698,6 +732,9 @@ impl HasAttrs for StmtKind { } impl HasAttrs for Stmt { + fn check_cfg_attrs(&self, handler: &Handler) { + self.kind.check_cfg_attrs(handler) + } fn attrs(&self) -> &[ast::Attribute] { self.kind.attrs() } @@ -717,6 +754,22 @@ impl HasAttrs for GenericParam { } } +impl HasAttrs for Expr { + fn check_cfg_attrs(&self, handler: &Handler) { + debug!("check_cfg_attrs: Expr {:?}", self); + if let ExprKind::If(..) = &self.kind { + check_attr_on_if_expr(&self.attrs, handler); + } + } + fn attrs(&self) -> &[Attribute] { + &self.attrs + } + + fn visit_attrs)>(&mut self, f: F) { + self.attrs.visit_attrs(f); + } +} + macro_rules! derive_has_attrs { ($($ty:path),*) => { $( impl HasAttrs for $ty { @@ -732,6 +785,6 @@ macro_rules! derive_has_attrs { } derive_has_attrs! { - Item, Expr, Local, ast::ForeignItem, ast::StructField, ast::AssocItem, ast::Arm, + Item, Local, ast::ForeignItem, ast::StructField, ast::AssocItem, ast::Arm, ast::Field, ast::FieldPat, ast::Variant, ast::Param } diff --git a/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs b/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs index 361fddd4ca3a3..4f8d2f775b373 100644 --- a/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs +++ b/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs @@ -1,4 +1,11 @@ fn main() { + + #[cfg(FALSE)] //~ ERROR attributes are not yet allowed on `if` expressions` + if true { panic!() } + + #[cfg_attr(FALSE, allow(warnings))] //~ ERROR attributes are not yet allowed on `if` expressions` + if true { panic!() } + #[allow(warnings)] if true {} //~ ERROR attributes are not yet allowed on `if` expressions if false { } else if true { diff --git a/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr b/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr index 3326d05dbda1c..06e4c95fa42ae 100644 --- a/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr +++ b/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr @@ -1,11 +1,11 @@ error: attributes are not yet allowed on `if` expressions - --> $DIR/deny-if-attr-validation.rs:2:5 + --> $DIR/deny-if-attr-validation.rs:9:5 | LL | #[allow(warnings)] if true {} | ^^^^^^^^^^^^^^^^^^ error: attributes are not yet allowed on `if` expressions - --> $DIR/deny-if-attr-validation.rs:7:5 + --> $DIR/deny-if-attr-validation.rs:14:5 | LL | #[allow(warnings)] if let Some(_) = Some(true) { | ^^^^^^^^^^^^^^^^^^ From ed3474d6f017678aec2355056b830cfc27f20941 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 21:26:57 -0500 Subject: [PATCH 11/13] De-duplicate errors --- src/libsyntax/attr/mod.rs | 14 ++++++++++---- src/libsyntax/lib.rs | 5 +++++ .../ui/parser/if-attrs/deny-if-attr-validation.rs | 4 ++-- .../parser/if-attrs/deny-if-attr-validation.stderr | 14 +++++++++++++- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index 1bca95fcdc814..36db05d14c8bf 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -32,10 +32,16 @@ use std::ops::DerefMut; /// the feature-gate check. pub fn check_attr_on_if_expr(attrs: &[Attribute], err_handler: &Handler) { if let [a0, ..] = attrs { - // Just point to the first attribute in there... - err_handler - .struct_span_err(a0.span, "attributes are not yet allowed on `if` expressions") - .emit(); + // Deduplicate errors by attr id, as this method may get multiple times + // for the same set of attrs + if GLOBALS.with(|globals| { + globals.if_expr_attrs.lock().insert(a0.id) + }) { + // Just point to the first attribute in there... + err_handler + .struct_span_err(a0.span, "attributes are not yet allowed on `if` expressions") + .emit(); + } } } diff --git a/src/libsyntax/lib.rs b/src/libsyntax/lib.rs index b0c2aa3dbb28e..1e88e388ed33e 100644 --- a/src/libsyntax/lib.rs +++ b/src/libsyntax/lib.rs @@ -35,6 +35,10 @@ macro_rules! unwrap_or { pub struct Globals { used_attrs: Lock>, known_attrs: Lock>, + /// Stores attributes on 'if' expressions that + /// we've already emitted an error for, to avoid emitting + /// duplicate errors + if_expr_attrs: Lock>, rustc_span_globals: rustc_span::Globals, } @@ -45,6 +49,7 @@ impl Globals { // initiate the vectors with 0 bits. We'll grow them as necessary. used_attrs: Lock::new(GrowableBitSet::new_empty()), known_attrs: Lock::new(GrowableBitSet::new_empty()), + if_expr_attrs: Lock::new(GrowableBitSet::new_empty()), rustc_span_globals: rustc_span::Globals::new(edition), } } diff --git a/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs b/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs index 4f8d2f775b373..9caf7b0f89574 100644 --- a/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs +++ b/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs @@ -1,9 +1,9 @@ fn main() { - #[cfg(FALSE)] //~ ERROR attributes are not yet allowed on `if` expressions` + #[cfg(FALSE)] //~ ERROR attributes are not yet allowed on `if` expressions if true { panic!() } - #[cfg_attr(FALSE, allow(warnings))] //~ ERROR attributes are not yet allowed on `if` expressions` + #[cfg_attr(FALSE, allow(warnings))] //~ ERROR attributes are not yet allowed on `if` expressions if true { panic!() } #[allow(warnings)] if true {} //~ ERROR attributes are not yet allowed on `if` expressions diff --git a/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr b/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr index 06e4c95fa42ae..08684530c3449 100644 --- a/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr +++ b/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr @@ -1,3 +1,15 @@ +error: attributes are not yet allowed on `if` expressions + --> $DIR/deny-if-attr-validation.rs:3:5 + | +LL | #[cfg(FALSE)] + | ^^^^^^^^^^^^^ + +error: attributes are not yet allowed on `if` expressions + --> $DIR/deny-if-attr-validation.rs:6:5 + | +LL | #[cfg_attr(FALSE, allow(warnings))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + error: attributes are not yet allowed on `if` expressions --> $DIR/deny-if-attr-validation.rs:9:5 | @@ -10,5 +22,5 @@ error: attributes are not yet allowed on `if` expressions LL | #[allow(warnings)] if let Some(_) = Some(true) { | ^^^^^^^^^^^^^^^^^^ -error: aborting due to 2 previous errors +error: aborting due to 4 previous errors From 6c12e4c5e3999b8931af4477a0612cb95281050c Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 21:43:08 -0500 Subject: [PATCH 12/13] Run fmt --- src/libsyntax/attr/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index 36db05d14c8bf..bde5edc15a971 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -18,9 +18,9 @@ use crate::token::{self, Token}; use crate::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint}; use crate::GLOBALS; +use rustc_errors::Handler; use rustc_span::source_map::{BytePos, Spanned}; use rustc_span::symbol::{sym, Symbol}; -use rustc_errors::Handler; use rustc_span::Span; use log::debug; @@ -34,15 +34,13 @@ pub fn check_attr_on_if_expr(attrs: &[Attribute], err_handler: &Handler) { if let [a0, ..] = attrs { // Deduplicate errors by attr id, as this method may get multiple times // for the same set of attrs - if GLOBALS.with(|globals| { - globals.if_expr_attrs.lock().insert(a0.id) - }) { + if GLOBALS.with(|globals| globals.if_expr_attrs.lock().insert(a0.id)) { // Just point to the first attribute in there... err_handler .struct_span_err(a0.span, "attributes are not yet allowed on `if` expressions") .emit(); } - } + } } pub fn mark_used(attr: &Attribute) { @@ -706,7 +704,7 @@ impl HasAttrs for StmtKind { match *self { StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => { expr.check_cfg_attrs(handler); - }, + } _ => {} } } From b36d4d9516cf96cf56661779e774f573303c5a32 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 29 Jan 2020 21:53:50 -0500 Subject: [PATCH 13/13] Fix tests --- src/test/ui/parser/if-attrs/cfg-false-if-attr.rs | 5 ----- .../parser/if-attrs/deny-if-attr-validation.rs | 6 ++++++ .../if-attrs/deny-if-attr-validation.stderr | 16 +++++++++++----- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs b/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs index ca4c5cbbaf3ac..2932ec1a23195 100644 --- a/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs +++ b/src/test/ui/parser/if-attrs/cfg-false-if-attr.rs @@ -19,11 +19,6 @@ fn if_let() { #[attr] if let Some(_) = Some(true) {} } -fn bar() {} -fn foo() { - bar(#[cfg(FALSE)] if true {}); -} - macro_rules! custom_macro { ($expr:expr) => {} } diff --git a/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs b/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs index 9caf7b0f89574..5ff02b5d612b0 100644 --- a/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs +++ b/src/test/ui/parser/if-attrs/deny-if-attr-validation.rs @@ -1,3 +1,9 @@ +fn bar() {} +fn foo() { + bar(#[cfg(FALSE)] if true {}); //~ ERROR attributes are not yet allowed +} + + fn main() { #[cfg(FALSE)] //~ ERROR attributes are not yet allowed on `if` expressions diff --git a/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr b/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr index 08684530c3449..0abfc245aa047 100644 --- a/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr +++ b/src/test/ui/parser/if-attrs/deny-if-attr-validation.stderr @@ -1,26 +1,32 @@ error: attributes are not yet allowed on `if` expressions - --> $DIR/deny-if-attr-validation.rs:3:5 + --> $DIR/deny-if-attr-validation.rs:3:9 + | +LL | bar(#[cfg(FALSE)] if true {}); + | ^^^^^^^^^^^^^ + +error: attributes are not yet allowed on `if` expressions + --> $DIR/deny-if-attr-validation.rs:9:5 | LL | #[cfg(FALSE)] | ^^^^^^^^^^^^^ error: attributes are not yet allowed on `if` expressions - --> $DIR/deny-if-attr-validation.rs:6:5 + --> $DIR/deny-if-attr-validation.rs:12:5 | LL | #[cfg_attr(FALSE, allow(warnings))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: attributes are not yet allowed on `if` expressions - --> $DIR/deny-if-attr-validation.rs:9:5 + --> $DIR/deny-if-attr-validation.rs:15:5 | LL | #[allow(warnings)] if true {} | ^^^^^^^^^^^^^^^^^^ error: attributes are not yet allowed on `if` expressions - --> $DIR/deny-if-attr-validation.rs:14:5 + --> $DIR/deny-if-attr-validation.rs:20:5 | LL | #[allow(warnings)] if let Some(_) = Some(true) { | ^^^^^^^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors