Disallow ambiguous attributes on expressions

Author: voidc

Diff for: compiler/rustc_parse/messages.ftl

@@ -620,6 +620,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

Diff for: compiler/rustc_parse/src/errors.rs

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

Diff for: compiler/rustc_parse/src/parser/expr.rs

@@ -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
@@ -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.
     ///
@@ -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);
@@ -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
@@ -3807,16 +3823,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,

Diff for: src/tools/clippy/tests/ui/cfg_attr_rustfmt.fixed

@@ -16,7 +16,7 @@ fn foo(
 
 fn skip_on_statements() {
     #[rustfmt::skip]
-    5+3;
+    { 5+3; }
 }
 
 #[rustfmt::skip]
@@ -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; }
 }

Diff for: src/tools/clippy/tests/ui/cfg_attr_rustfmt.rs

@@ -16,7 +16,7 @@ fn foo(
 
 fn skip_on_statements() {
     #[cfg_attr(rustfmt, rustfmt::skip)]
-    5+3;
+    { 5+3; }
 }
 
 #[cfg_attr(rustfmt, rustfmt_skip)]
@@ -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; }
 }

Diff for: src/tools/rustfmt/tests/source/attrib.rs

@@ -214,8 +214,8 @@ type Os = NoSource;
 // #3313
 fn stmt_expr_attributes() {
     let foo ;
-    #[must_use]
-   foo = false ;
+    (#[must_use]
+   foo) = false ;
 }
 
 // #3509

Diff for: src/tools/rustfmt/tests/target/attrib.rs

@@ -248,8 +248,8 @@ type Os = NoSource;
 // #3313
 fn stmt_expr_attributes() {
     let foo;
-    #[must_use]
-    foo = false;
+    (#[must_use]
+    foo) = false;
 }
 
 // #3509

Diff for: tests/pretty/ast-stmt-expr-attr.rs

@@ -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 {
@@ -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);

Diff for: tests/pretty/stmt_expr_attributes.rs

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

Diff for: tests/ui/lint/unused/unused-doc-comments-edge-cases.rs

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

Diff for: tests/ui/lint/unused/unused-doc-comments-edge-cases.stderr

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

Diff for: tests/ui/parser/attribute/attr-binary-expr-ambigous.fixed

@@ -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);
+}

Diff for: tests/ui/parser/attribute/attr-binary-expr-ambigous.rs

@@ -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);
+}

Diff for: tests/ui/parser/attribute/attr-binary-expr-ambigous.stderr

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

Diff for: tests/ui/proc-macro/issue-81555.rs

@@ -10,6 +10,5 @@ use test_macros::identity_attr;
 fn main() {
     let _x;
     let y = ();
-    #[identity_attr]
-    _x = y;
+    (#[identity_attr] _x) = y;
 }