Skip to content

Commit

Permalink
Rollup merge of rust-lang#124099 - voidc:disallow-ambiguous-expr-attr…
Browse files Browse the repository at this point in the history
…s, r=davidtwco

Disallow ambiguous attributes on expressions

This implements the suggestion in [rust-lang#15701](rust-lang#15701 (comment)) to disallow ambiguous outer attributes on expressions. This should resolve one of the concerns blocking the stabilization of `stmt_expr_attributes`.
  • Loading branch information
matthiaskrgr authored Apr 23, 2024
2 parents 36316df + 5af861c commit 57dad1d
Show file tree
Hide file tree
Showing 15 changed files with 141 additions and 49 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,8 @@ parse_or_pattern_not_allowed_in_let_binding = top-level or-patterns are not allo
parse_out_of_range_hex_escape = out of range hex escape
.label = must be a character in the range [\x00-\x7f]
parse_outer_attr_ambiguous = ambiguous outer attributes
parse_outer_attr_explanation = outer attributes, like `#[test]`, annotate the item following them
parse_outer_attribute_not_allowed_on_if_else = outer attributes are not allowed on `if` and `else` branches
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,15 @@ pub(crate) struct OuterAttributeNotAllowedOnIfElse {
pub attributes: Span,
}

#[derive(Diagnostic)]
#[diag(parse_outer_attr_ambiguous)]
pub(crate) struct AmbiguousOuterAttributes {
#[primary_span]
pub span: Span,
#[subdiagnostic]
pub sugg: WrapInParentheses,
}

#[derive(Diagnostic)]
#[diag(parse_missing_in_in_for_loop)]
pub(crate) struct MissingInInForLoop {
Expand Down
32 changes: 19 additions & 13 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,9 @@ impl<'a> Parser<'a> {
this.parse_expr_assoc_with(prec + prec_adjustment, LhsExpr::NotYetParsed)
})?;

let span = self.mk_expr_sp(&lhs, lhs_span, rhs.span);
self.error_ambiguous_outer_attrs(&lhs, lhs_span, rhs.span);
let span = lhs_span.to(rhs.span);

lhs = match op {
AssocOp::Add
| AssocOp::Subtract
Expand Down Expand Up @@ -426,6 +428,18 @@ impl<'a> Parser<'a> {
});
}

fn error_ambiguous_outer_attrs(&self, lhs: &P<Expr>, lhs_span: Span, rhs_span: Span) {
if let Some(attr) = lhs.attrs.iter().find(|a| a.style == AttrStyle::Outer) {
self.dcx().emit_err(errors::AmbiguousOuterAttributes {
span: attr.span.to(rhs_span),
sugg: errors::WrapInParentheses::Expression {
left: attr.span.shrink_to_lo(),
right: lhs_span.shrink_to_hi(),
},
});
}
}

/// Possibly translate the current token to an associative operator.
/// The method does not advance the current token.
///
Expand Down Expand Up @@ -506,7 +520,8 @@ impl<'a> Parser<'a> {
None
};
let rhs_span = rhs.as_ref().map_or(cur_op_span, |x| x.span);
let span = self.mk_expr_sp(&lhs, lhs.span, rhs_span);
self.error_ambiguous_outer_attrs(&lhs, lhs.span, rhs_span);
let span = lhs.span.to(rhs_span);
let limits =
if op == AssocOp::DotDot { RangeLimits::HalfOpen } else { RangeLimits::Closed };
let range = self.mk_range(Some(lhs), rhs, limits);
Expand Down Expand Up @@ -722,7 +737,8 @@ impl<'a> Parser<'a> {
expr_kind: fn(P<Expr>, P<Ty>) -> ExprKind,
) -> PResult<'a, P<Expr>> {
let mk_expr = |this: &mut Self, lhs: P<Expr>, rhs: P<Ty>| {
this.mk_expr(this.mk_expr_sp(&lhs, lhs_span, rhs.span), expr_kind(lhs, rhs))
this.error_ambiguous_outer_attrs(&lhs, lhs_span, rhs.span);
this.mk_expr(lhs_span.to(rhs.span), expr_kind(lhs, rhs))
};

// Save the state of the parser before parsing type normally, in case there is a
Expand Down Expand Up @@ -3813,16 +3829,6 @@ impl<'a> Parser<'a> {
self.mk_expr(span, ExprKind::Err(guar))
}

/// Create expression span ensuring the span of the parent node
/// is larger than the span of lhs and rhs, including the attributes.
fn mk_expr_sp(&self, lhs: &P<Expr>, lhs_span: Span, rhs_span: Span) -> Span {
lhs.attrs
.iter()
.find(|a| a.style == AttrStyle::Outer)
.map_or(lhs_span, |a| a.span)
.to(rhs_span)
}

fn collect_tokens_for_expr(
&mut self,
attrs: AttrWrapper,
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/tests/ui/cfg_attr_rustfmt.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn foo(

fn skip_on_statements() {
#[rustfmt::skip]
5+3;
{ 5+3; }
}

#[rustfmt::skip]
Expand All @@ -33,11 +33,11 @@ mod foo {
#[clippy::msrv = "1.29"]
fn msrv_1_29() {
#[cfg_attr(rustfmt, rustfmt::skip)]
1+29;
{ 1+29; }
}

#[clippy::msrv = "1.30"]
fn msrv_1_30() {
#[rustfmt::skip]
1+30;
{ 1+30; }
}
6 changes: 3 additions & 3 deletions src/tools/clippy/tests/ui/cfg_attr_rustfmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn foo(

fn skip_on_statements() {
#[cfg_attr(rustfmt, rustfmt::skip)]
5+3;
{ 5+3; }
}

#[cfg_attr(rustfmt, rustfmt_skip)]
Expand All @@ -33,11 +33,11 @@ mod foo {
#[clippy::msrv = "1.29"]
fn msrv_1_29() {
#[cfg_attr(rustfmt, rustfmt::skip)]
1+29;
{ 1+29; }
}

#[clippy::msrv = "1.30"]
fn msrv_1_30() {
#[cfg_attr(rustfmt, rustfmt::skip)]
1+30;
{ 1+30; }
}
4 changes: 2 additions & 2 deletions src/tools/rustfmt/tests/source/attrib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ type Os = NoSource;
// #3313
fn stmt_expr_attributes() {
let foo ;
#[must_use]
foo = false ;
(#[must_use]
foo) = false ;
}

// #3509
Expand Down
4 changes: 2 additions & 2 deletions src/tools/rustfmt/tests/target/attrib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ type Os = NoSource;
// #3313
fn stmt_expr_attributes() {
let foo;
#[must_use]
foo = false;
(#[must_use]
foo) = false;
}

// #3509
Expand Down
18 changes: 9 additions & 9 deletions tests/pretty/ast-stmt-expr-attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ fn syntax() {
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;
let _ = #[attr] 0 % #[attr] 0;
let _ = (#[attr] 0) + #[attr] 0;
let _ = (#[attr] 0) / #[attr] 0;
let _ = (#[attr] 0) & #[attr] 0;
let _ = (#[attr] 0) % #[attr] 0;
let _ = #[attr] (0 + 0);
let _ = #[attr] !0;
let _ = #[attr] -0;
let _ = #[attr] false;
let _ = #[attr] 0;
let _ = #[attr] 'c';
let _ = #[attr] x as Y;
let _ = (#[attr] x) as Y;
let _ = #[attr] (x as Y);
let _ =
#[attr] while true {
Expand Down Expand Up @@ -88,18 +88,18 @@ fn syntax() {
let _ = ();
foo
};
let _ = #[attr] x = y;
let _ = (#[attr] x) = y;
let _ = #[attr] (x = y);
let _ = #[attr] x += y;
let _ = (#[attr] x) += y;
let _ = #[attr] (x += y);
let _ = #[attr] foo.bar;
let _ = (#[attr] foo).bar;
let _ = #[attr] foo.0;
let _ = (#[attr] foo).0;
let _ = #[attr] foo[bar];
let _ = (#[attr] foo)[bar];
let _ = #[attr] 0..#[attr] 0;
let _ = #[attr] 0..;
let _ = (#[attr] 0)..#[attr] 0;
let _ = (#[attr] 0)..;
let _ = #[attr] (0..0);
let _ = #[attr] (0..);
let _ = #[attr] (..0);
Expand Down
12 changes: 6 additions & 6 deletions tests/pretty/stmt_expr_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,13 @@ fn _11() {
let _ = #[rustc_dummy] (0);
let _ = #[rustc_dummy] (0,);
let _ = #[rustc_dummy] (0, 0);
let _ = #[rustc_dummy] 0 + #[rustc_dummy] 0;
let _ = (#[rustc_dummy] 0) + #[rustc_dummy] 0;
let _ = #[rustc_dummy] !0;
let _ = #[rustc_dummy] -0i32;
let _ = #[rustc_dummy] false;
let _ = #[rustc_dummy] 'c';
let _ = #[rustc_dummy] 0;
let _ = #[rustc_dummy] 0 as usize;
let _ = (#[rustc_dummy] 0) as usize;
let _ =
#[rustc_dummy] while false {
#![rustc_dummy]
Expand Down Expand Up @@ -214,8 +214,8 @@ fn _11() {
#![rustc_dummy]
};
let mut x = 0;
let _ = #[rustc_dummy] x = 15;
let _ = #[rustc_dummy] x += 15;
let _ = (#[rustc_dummy] x) = 15;
let _ = (#[rustc_dummy] x) += 15;
let s = Foo { data: () };
let _ = #[rustc_dummy] s.data;
let _ = (#[rustc_dummy] s).data;
Expand All @@ -225,8 +225,8 @@ fn _11() {
let v = vec!(0);
let _ = #[rustc_dummy] v[0];
let _ = (#[rustc_dummy] v)[0];
let _ = #[rustc_dummy] 0..#[rustc_dummy] 0;
let _ = #[rustc_dummy] 0..;
let _ = (#[rustc_dummy] 0)..#[rustc_dummy] 0;
let _ = (#[rustc_dummy] 0)..;
let _ = #[rustc_dummy] (0..0);
let _ = #[rustc_dummy] (0..);
let _ = #[rustc_dummy] (..0);
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/lint/unused/unused-doc-comments-edge-cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ fn doc_comment_between_if_else(num: u8) -> bool {
}

fn doc_comment_on_expr(num: u8) -> bool {
/// useless doc comment
(/// useless doc comment
//~^ ERROR: attributes on expressions are experimental
//~| ERROR: unused doc comment
num == 3
num) == 3
}

fn doc_comment_on_expr_field() -> bool {
Expand Down
14 changes: 7 additions & 7 deletions tests/ui/lint/unused/unused-doc-comments-edge-cases.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ LL | else {
| ^^^^ expected expression

error[E0658]: attributes on expressions are experimental
--> $DIR/unused-doc-comments-edge-cases.rs:23:5
--> $DIR/unused-doc-comments-edge-cases.rs:23:6
|
LL | /// useless doc comment
| ^^^^^^^^^^^^^^^^^^^^^^^
LL | (/// useless doc comment
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
Expand All @@ -32,12 +32,12 @@ LL | #![deny(unused_doc_comments)]
| ^^^^^^^^^^^^^^^^^^^

error: unused doc comment
--> $DIR/unused-doc-comments-edge-cases.rs:23:5
--> $DIR/unused-doc-comments-edge-cases.rs:23:6
|
LL | /// useless doc comment
| ^^^^^^^^^^^^^^^^^^^^^^^
LL | (/// useless doc comment
| ^^^^^^^^^^^^^^^^^^^^^^^
...
LL | num == 3
LL | num) == 3
| --- rustdoc does not generate documentation for expressions
|
= help: use `//` for a plain comment
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/parser/attribute/attr-binary-expr-ambigous.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//@ run-rustfix
#![feature(stmt_expr_attributes)]
#![allow(unused_assignments, unused_attributes)]

fn main() {
let mut x = (#[deprecated] 1) + 2; //~ ERROR ambiguous

(#[deprecated] x) = 4; //~ ERROR ambiguous

x = (#[deprecated] 5) as i32; //~ ERROR ambiguous

let _r = (#[deprecated] 1)..6; //~ ERROR ambiguous

println!("{}", x);
}
15 changes: 15 additions & 0 deletions tests/ui/parser/attribute/attr-binary-expr-ambigous.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//@ run-rustfix
#![feature(stmt_expr_attributes)]
#![allow(unused_assignments, unused_attributes)]

fn main() {
let mut x = #[deprecated] 1 + 2; //~ ERROR ambiguous

#[deprecated] x = 4; //~ ERROR ambiguous

x = #[deprecated] 5 as i32; //~ ERROR ambiguous

let _r = #[deprecated] 1..6; //~ ERROR ambiguous

println!("{}", x);
}
46 changes: 46 additions & 0 deletions tests/ui/parser/attribute/attr-binary-expr-ambigous.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
error: ambiguous outer attributes
--> $DIR/attr-binary-expr-ambigous.rs:6:17
|
LL | let mut x = #[deprecated] 1 + 2;
| ^^^^^^^^^^^^^^^^^^^
|
help: wrap the expression in parentheses
|
LL | let mut x = (#[deprecated] 1) + 2;
| + +

error: ambiguous outer attributes
--> $DIR/attr-binary-expr-ambigous.rs:8:5
|
LL | #[deprecated] x = 4;
| ^^^^^^^^^^^^^^^^^^^
|
help: wrap the expression in parentheses
|
LL | (#[deprecated] x) = 4;
| + +

error: ambiguous outer attributes
--> $DIR/attr-binary-expr-ambigous.rs:10:9
|
LL | x = #[deprecated] 5 as i32;
| ^^^^^^^^^^^^^^^^^^^^^^
|
help: wrap the expression in parentheses
|
LL | x = (#[deprecated] 5) as i32;
| + +

error: ambiguous outer attributes
--> $DIR/attr-binary-expr-ambigous.rs:12:14
|
LL | let _r = #[deprecated] 1..6;
| ^^^^^^^^^^^^^^^^^^
|
help: wrap the expression in parentheses
|
LL | let _r = (#[deprecated] 1)..6;
| + +

error: aborting due to 4 previous errors

3 changes: 1 addition & 2 deletions tests/ui/proc-macro/issue-81555.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@ use test_macros::identity_attr;
fn main() {
let _x;
let y = ();
#[identity_attr]
_x = y;
(#[identity_attr] _x) = y;
}

0 comments on commit 57dad1d

Please sign in to comment.