Skip to content

Commit

Permalink
Auto merge of #83312 - petrochenkov:noinner, r=Aaron1011
Browse files Browse the repository at this point in the history
parser: Remove support for inner attributes on non-block expressions

Remove support for attributes like
```rust
fn attrs() {
    (#![print_target_and_args(fifth)] 1, 2);

    [#![print_target_and_args(sixth)] 1 , 2];
    [#![print_target_and_args(seventh)] true ; 5];

    match 0 {
        #![print_target_and_args(eighth)]
        _ => {}
    }

    MyStruct { #![print_target_and_args(ninth)] field: true };
}
```
They are
- useless
- unstable (modulo holes like #65860)
- pessimize compiler performance, namely token collection for macros (cc #82608)

I still want to run crater on this to check whether the stability holes are exploited in practice, and whether such attributes are used at all.
  • Loading branch information
bors committed May 3, 2021
2 parents e327a82 + 1443c76 commit c825bc4
Show file tree
Hide file tree
Showing 13 changed files with 316 additions and 637 deletions.
35 changes: 9 additions & 26 deletions compiler/rustc_ast_pretty/src/pprust/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,10 +366,6 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
self.print_either_attributes(attrs, ast::AttrStyle::Inner, false, true)
}

fn print_inner_attributes_no_trailing_hardbreak(&mut self, attrs: &[ast::Attribute]) {
self.print_either_attributes(attrs, ast::AttrStyle::Inner, false, false)
}

fn print_outer_attributes(&mut self, attrs: &[ast::Attribute]) {
self.print_either_attributes(attrs, ast::AttrStyle::Outer, false, true)
}
Expand Down Expand Up @@ -1675,32 +1671,24 @@ impl<'a> State<'a> {
}
}

fn print_expr_vec(&mut self, exprs: &[P<ast::Expr>], attrs: &[ast::Attribute]) {
fn print_expr_vec(&mut self, exprs: &[P<ast::Expr>]) {
self.ibox(INDENT_UNIT);
self.s.word("[");
self.print_inner_attributes_inline(attrs);
self.commasep_exprs(Inconsistent, exprs);
self.s.word("]");
self.end();
}

fn print_expr_anon_const(&mut self, expr: &ast::AnonConst, attrs: &[ast::Attribute]) {
fn print_expr_anon_const(&mut self, expr: &ast::AnonConst) {
self.ibox(INDENT_UNIT);
self.s.word("const");
self.print_inner_attributes_inline(attrs);
self.print_expr(&expr.value);
self.end();
}

fn print_expr_repeat(
&mut self,
element: &ast::Expr,
count: &ast::AnonConst,
attrs: &[ast::Attribute],
) {
fn print_expr_repeat(&mut self, element: &ast::Expr, count: &ast::AnonConst) {
self.ibox(INDENT_UNIT);
self.s.word("[");
self.print_inner_attributes_inline(attrs);
self.print_expr(element);
self.word_space(";");
self.print_expr(&count.value);
Expand All @@ -1713,11 +1701,9 @@ impl<'a> State<'a> {
path: &ast::Path,
fields: &[ast::ExprField],
rest: &ast::StructRest,
attrs: &[ast::Attribute],
) {
self.print_path(path, true, 0);
self.s.word("{");
self.print_inner_attributes_inline(attrs);
self.commasep_cmnt(
Consistent,
fields,
Expand Down Expand Up @@ -1752,9 +1738,8 @@ impl<'a> State<'a> {
self.s.word("}");
}

fn print_expr_tup(&mut self, exprs: &[P<ast::Expr>], attrs: &[ast::Attribute]) {
fn print_expr_tup(&mut self, exprs: &[P<ast::Expr>]) {
self.popen();
self.print_inner_attributes_inline(attrs);
self.commasep_exprs(Inconsistent, exprs);
if exprs.len() == 1 {
self.s.word(",");
Expand Down Expand Up @@ -1865,19 +1850,19 @@ impl<'a> State<'a> {
self.print_expr_maybe_paren(expr, parser::PREC_PREFIX);
}
ast::ExprKind::Array(ref exprs) => {
self.print_expr_vec(&exprs[..], attrs);
self.print_expr_vec(exprs);
}
ast::ExprKind::ConstBlock(ref anon_const) => {
self.print_expr_anon_const(anon_const, attrs);
self.print_expr_anon_const(anon_const);
}
ast::ExprKind::Repeat(ref element, ref count) => {
self.print_expr_repeat(element, count, attrs);
self.print_expr_repeat(element, count);
}
ast::ExprKind::Struct(ref se) => {
self.print_expr_struct(&se.path, &se.fields, &se.rest, attrs);
self.print_expr_struct(&se.path, &se.fields, &se.rest);
}
ast::ExprKind::Tup(ref exprs) => {
self.print_expr_tup(&exprs[..], attrs);
self.print_expr_tup(exprs);
}
ast::ExprKind::Call(ref func, ref args) => {
self.print_expr_call(func, &args[..]);
Expand Down Expand Up @@ -1955,7 +1940,6 @@ impl<'a> State<'a> {
self.print_expr_as_cond(expr);
self.s.space();
self.bopen();
self.print_inner_attributes_no_trailing_hardbreak(attrs);
for arm in arms {
self.print_arm(arm);
}
Expand Down Expand Up @@ -2253,7 +2237,6 @@ impl<'a> State<'a> {
ast::ExprKind::MacCall(ref m) => self.print_mac(m),
ast::ExprKind::Paren(ref e) => {
self.popen();
self.print_inner_attributes_inline(attrs);
self.print_expr(e);
self.pclose();
}
Expand Down
14 changes: 4 additions & 10 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1216,10 +1216,9 @@ impl<'a> Parser<'a> {
}
}

fn parse_tuple_parens_expr(&mut self, mut attrs: AttrVec) -> PResult<'a, P<Expr>> {
fn parse_tuple_parens_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
let lo = self.token.span;
self.expect(&token::OpenDelim(token::Paren))?;
attrs.extend(self.parse_inner_attributes()?); // `(#![foo] a, b, ...)` is OK.
let (es, trailing_comma) = match self.parse_seq_to_end(
&token::CloseDelim(token::Paren),
SeqSep::trailing_allowed(token::Comma),
Expand All @@ -1239,12 +1238,10 @@ impl<'a> Parser<'a> {
self.maybe_recover_from_bad_qpath(expr, true)
}

fn parse_array_or_repeat_expr(&mut self, mut attrs: AttrVec) -> PResult<'a, P<Expr>> {
fn parse_array_or_repeat_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
let lo = self.token.span;
self.bump(); // `[`

attrs.extend(self.parse_inner_attributes()?);

let close = &token::CloseDelim(token::Bracket);
let kind = if self.eat(close) {
// Empty vector
Expand Down Expand Up @@ -1950,7 +1947,7 @@ impl<'a> Parser<'a> {
}

/// Parses a `match ... { ... }` expression (`match` token already eaten).
fn parse_match_expr(&mut self, mut attrs: AttrVec) -> PResult<'a, P<Expr>> {
fn parse_match_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
let match_span = self.prev_token.span;
let lo = self.prev_token.span;
let scrutinee = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?;
Expand All @@ -1965,7 +1962,6 @@ impl<'a> Parser<'a> {
}
return Err(e);
}
attrs.extend(self.parse_inner_attributes()?);

let mut arms: Vec<Arm> = Vec::new();
while self.token != token::CloseDelim(token::Brace) {
Expand Down Expand Up @@ -2293,15 +2289,13 @@ impl<'a> Parser<'a> {
pub(super) fn parse_struct_expr(
&mut self,
pth: ast::Path,
mut attrs: AttrVec,
attrs: AttrVec,
recover: bool,
) -> PResult<'a, P<Expr>> {
let mut fields = Vec::new();
let mut base = ast::StructRest::None;
let mut recover_async = false;

attrs.extend(self.parse_inner_attributes()?);

let mut async_block_err = |e: &mut DiagnosticBuilder<'_>, span: Span| {
recover_async = true;
e.span_label(span, "`async` blocks are only allowed in Rust 2018 or later");
Expand Down
30 changes: 15 additions & 15 deletions src/test/pretty/ast-stmt-expr-attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ fn main() { }
#[cfg(FALSE)]
fn syntax() {
let _ = #[attr] box 0;
let _ = #[attr] [#![attr] ];
let _ = #[attr] [#![attr] 0];
let _ = #[attr] [#![attr] 0; 0];
let _ = #[attr] [#![attr] 0, 0, 0];
let _ = #[attr] [];
let _ = #[attr] [0];
let _ = #[attr] [0; 0];
let _ = #[attr] [0, 0, 0];
let _ = #[attr] foo();
let _ = #[attr] x.foo();
let _ = #[attr] (#![attr] );
let _ = #[attr] (#![attr] #[attr] 0,);
let _ = #[attr] (#![attr] #[attr] 0, 0);
let _ = #[attr] ();
let _ = #[attr] (#[attr] 0,);
let _ = #[attr] (#[attr] 0, 0);
let _ = #[attr] 0 + #[attr] 0;
let _ = #[attr] 0 / #[attr] 0;
let _ = #[attr] 0 & #[attr] 0;
Expand Down Expand Up @@ -43,10 +43,10 @@ fn syntax() {
#![attr]
};
let _ =
#[attr] match true {
#![attr]
#[attr]
_ => false,
#[attr] match true
{
#[attr]
_ => false,
};
let _ = #[attr] || #[attr] foo;
let _ = #[attr] move || #[attr] foo;
Expand Down Expand Up @@ -119,10 +119,10 @@ fn syntax() {
let _ = #[attr] foo![# ! [attr]];
let _ = #[attr] foo! { };
let _ = #[attr] foo! { # ! [attr] };
let _ = #[attr] Foo{#![attr] bar: baz,};
let _ = #[attr] Foo{#![attr] ..foo};
let _ = #[attr] Foo{#![attr] bar: baz, ..foo};
let _ = #[attr] (#![attr] 0);
let _ = #[attr] Foo{bar: baz,};
let _ = #[attr] Foo{..foo};
let _ = #[attr] Foo{bar: baz, ..foo};
let _ = #[attr] (0);

{
#[attr]
Expand Down
59 changes: 24 additions & 35 deletions src/test/pretty/stmt_expr_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,9 @@ fn _3() {
fn _4() {

#[rustc_dummy]
match () {
#![rustc_dummy]
_ => (),
}
match () { _ => (), }

let _ =
#[rustc_dummy] match () {
#![rustc_dummy]
() => (),
};
let _ = #[rustc_dummy] match () { () => (), };
}

fn _5() {
Expand All @@ -71,14 +64,14 @@ fn _5() {
fn _6() {

#[rustc_dummy]
[#![rustc_dummy] 1, 2, 3];
[1, 2, 3];

let _ = #[rustc_dummy] [#![rustc_dummy] 1, 2, 3];
let _ = #[rustc_dummy] [1, 2, 3];

#[rustc_dummy]
[#![rustc_dummy] 1; 4];
[1; 4];

let _ = #[rustc_dummy] [#![rustc_dummy] 1; 4];
let _ = #[rustc_dummy] [1; 4];
}

struct Foo {
Expand All @@ -90,24 +83,24 @@ struct Bar(());
fn _7() {

#[rustc_dummy]
Foo{#![rustc_dummy] data: (),};
Foo{data: (),};

let _ = #[rustc_dummy] Foo{#![rustc_dummy] data: (),};
let _ = #[rustc_dummy] Foo{data: (),};
}

fn _8() {

#[rustc_dummy]
(#![rustc_dummy] );
();

#[rustc_dummy]
(#![rustc_dummy] 0);
(0);

#[rustc_dummy]
(#![rustc_dummy] 0,);
(0,);

#[rustc_dummy]
(#![rustc_dummy] 0, 1);
(0, 1);
}

fn _9() {
Expand Down Expand Up @@ -138,15 +131,15 @@ fn _10() {

fn _11() {
let _ = #[rustc_dummy] box 0;
let _: [(); 0] = #[rustc_dummy] [#![rustc_dummy] ];
let _ = #[rustc_dummy] [#![rustc_dummy] 0, 0];
let _ = #[rustc_dummy] [#![rustc_dummy] 0; 0];
let _: [(); 0] = #[rustc_dummy] [];
let _ = #[rustc_dummy] [0, 0];
let _ = #[rustc_dummy] [0; 0];
let _ = #[rustc_dummy] foo();
let _ = #[rustc_dummy] 1i32.clone();
let _ = #[rustc_dummy] (#![rustc_dummy] );
let _ = #[rustc_dummy] (#![rustc_dummy] 0);
let _ = #[rustc_dummy] (#![rustc_dummy] 0,);
let _ = #[rustc_dummy] (#![rustc_dummy] 0, 0);
let _ = #[rustc_dummy] ();
let _ = #[rustc_dummy] (0);
let _ = #[rustc_dummy] (0,);
let _ = #[rustc_dummy] (0, 0);
let _ = #[rustc_dummy] 0 + #[rustc_dummy] 0;
let _ = #[rustc_dummy] !0;
let _ = #[rustc_dummy] -0i32;
Expand All @@ -171,11 +164,7 @@ fn _11() {
#[rustc_dummy] loop {
#![rustc_dummy]
};
let _ =
#[rustc_dummy] match false {
#![rustc_dummy]
_ => (),
};
let _ = #[rustc_dummy] match false { _ => (), };
let _ = #[rustc_dummy] || #[rustc_dummy] ();
let _ = #[rustc_dummy] move || #[rustc_dummy] ();
let _ =
Expand Down Expand Up @@ -237,10 +226,10 @@ fn _11() {
let _ = #[rustc_dummy] expr_mac!();
let _ = #[rustc_dummy] expr_mac![];
let _ = #[rustc_dummy] expr_mac! { };
let _ = #[rustc_dummy] Foo{#![rustc_dummy] data: (),};
let _ = #[rustc_dummy] Foo{#![rustc_dummy] ..s};
let _ = #[rustc_dummy] Foo{#![rustc_dummy] data: (), ..s};
let _ = #[rustc_dummy] (#![rustc_dummy] 0);
let _ = #[rustc_dummy] Foo{data: (),};
let _ = #[rustc_dummy] Foo{..s};
let _ = #[rustc_dummy] Foo{data: (), ..s};
let _ = #[rustc_dummy] (0);
}

fn _12() {
Expand Down
6 changes: 2 additions & 4 deletions src/test/ui/lint/expr_attr_paren_order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@ fn main() {
// Test that attributes on parens get concatenated
// in the expected order in the hir folder.

#[deny(non_snake_case)] (
#![allow(non_snake_case)]
#[deny(non_snake_case)] #[allow(non_snake_case)] (
{
let X = 0;
let _ = X;
}
);

#[allow(non_snake_case)] (
#![deny(non_snake_case)]
#[allow(non_snake_case)] #[deny(non_snake_case)] (
{
let X = 0; //~ ERROR snake case name
let _ = X;
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/lint/expr_attr_paren_order.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
error: variable `X` should have a snake case name
--> $DIR/expr_attr_paren_order.rs:19:17
--> $DIR/expr_attr_paren_order.rs:17:17
|
LL | let X = 0;
| ^ help: convert the identifier to snake case (notice the capitalization): `x`
|
note: the lint level is defined here
--> $DIR/expr_attr_paren_order.rs:17:17
--> $DIR/expr_attr_paren_order.rs:15:37
|
LL | #![deny(non_snake_case)]
| ^^^^^^^^^^^^^^
LL | #[allow(non_snake_case)] #[deny(non_snake_case)] (
| ^^^^^^^^^^^^^^

error: aborting due to previous error

Loading

0 comments on commit c825bc4

Please sign in to comment.