Disallow ambiguous attributes on expressions (take 2)

compiler/rustc_ast_passes/src/feature_gate.rs

@@ -460,6 +460,7 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session, features: &Features) {
         "`fn(#[rustc_splat] (a, ...))` is incomplete",
         "call as func((a, ...)) instead"
     );
+    gate_all!(stmt_expr_attributes, "attributes on some expressions are unstable");
     gate_all!(super_let, "`super let` is experimental");
     gate_all!(try_blocks_heterogeneous, "`try bikeshed` expression is experimental");
     gate_all!(unnamed_enum_variants, "unnamed enum variants are experimental");

compiler/rustc_parse/src/parser/expr.rs

@@ -134,6 +134,15 @@ impl<'a> Parser<'a> {
         self.parse_expr_assoc_rest(min_prec, false, lhs)
     }
 
+    pub(super) fn suspicious_attribute(&self, expr: &Expr, _descr: &str) {
+        for attr in &expr.attrs {
+            if attr.style == AttrStyle::Outer {
+                // self.dcx().span_err(attr.span, format!("suspicious attribute: {descr}"));
+                self.psess.gated_spans.gate(sym::stmt_expr_attributes, attr.span);
+            }
+        }
+    }
+
     /// Parses the rest of an associative expression (i.e. the part after the lhs) with operators
     /// of at least `min_prec` precedence. The `bool` in the return value indicates if something
     /// was actually parsed.
@@ -284,7 +293,11 @@ impl<'a> Parser<'a> {
                     let binary = self.mk_binary(respan(cur_op_span, ast_op), lhs, rhs);
                     self.mk_expr(span, binary)
                 }
-                AssocOp::Assign => self.mk_expr(span, ExprKind::Assign(lhs, rhs, cur_op_span)),
+                AssocOp::Assign => {
+                    self.suspicious_attribute(&lhs, "ExprKind::Assign lhs");
+                    self.suspicious_attribute(&rhs, "ExprKind::Assign rhs");
+                    self.mk_expr(span, ExprKind::Assign(lhs, rhs, cur_op_span))
+                }
                 AssocOp::AssignOp(aop) => {
                     let aopexpr = self.mk_assign_op(respan(cur_op_span, aop), lhs, rhs);
                     self.mk_expr(span, aopexpr)
@@ -648,6 +661,7 @@ impl<'a> Parser<'a> {
         expr_kind: fn(Box<Expr>, Box<Ty>) -> ExprKind,
     ) -> PResult<'a, Box<Expr>> {
         let mk_expr = |this: &mut Self, lhs: Box<Expr>, rhs: Box<Ty>| {
+            this.suspicious_attribute(&lhs, "ExprKind::Cast");
             this.mk_expr(this.mk_expr_sp(&lhs, lhs_span, op_span, rhs.span), expr_kind(lhs, rhs))
         };
 
@@ -831,6 +845,7 @@ impl<'a> Parser<'a> {
             self.expected_token_types.insert(TokenType::KwConst);
         }
 
+        self.suspicious_attribute(&expr, "ExprKind::AddrOf");
         Ok((span, ExprKind::AddrOf(borrow_kind, mutbl, expr)))
     }
 
@@ -884,6 +899,7 @@ impl<'a> Parser<'a> {
             };
             if has_question {
                 // `expr?`
+                // self.suspicious_attribute(&e, "ExprKind::Try");
                 e = self.mk_expr(lo.to(self.prev_token.span), ExprKind::Try(e));
                 continue;
             }
@@ -1228,6 +1244,7 @@ impl<'a> Parser<'a> {
                 suffix,
             });
         }
+        // self.suspicious_attribute(&base, "ExprKind::Field tuple");
         self.mk_expr(lo.to(ident_span), ExprKind::Field(base, Ident::new(field, ident_span)))
     }
 
@@ -1389,6 +1406,7 @@ impl<'a> Parser<'a> {
         if self.eat_keyword(exp!(Yield)) {
             let yield_span = self.prev_token.span;
             self.psess.gated_spans.gate(sym::yield_expr, yield_span);
+            // self.suspicious_attribute(&self_arg, "ExprKind::Yield postfix");
             return Ok(
                 self.mk_expr(lo.to(yield_span), ExprKind::Yield(YieldKind::Postfix(self_arg)))
             );
@@ -1404,6 +1422,7 @@ impl<'a> Parser<'a> {
             let args = self.parse_expr_paren_seq()?;
             let fn_span = fn_span_lo.to(self.prev_token.span);
             let span = lo.to(self.prev_token.span);
+            // self.suspicious_attribute(&self_arg, "ExprKind::MethodCall");
             Ok(self.mk_expr(
                 span,
                 ExprKind::MethodCall(Box::new(ast::MethodCall {
@@ -1423,6 +1442,7 @@ impl<'a> Parser<'a> {
                     .stash(seg.ident.span, StashKey::GenericInFieldExpr);
             }
 
+            // self.suspicious_attribute(&self_arg, "ExprKind::Field struct");
             Ok(self.mk_expr(span, ExprKind::Field(self_arg, seg.ident)))
         }
     }
@@ -1869,7 +1889,11 @@ impl<'a> Parser<'a> {
     /// Parse `"return" expr?`.
     fn parse_expr_return(&mut self) -> PResult<'a, Box<Expr>> {
         let lo = self.prev_token.span;
-        let kind = ExprKind::Ret(self.parse_expr_opt()?);
+        let expr = self.parse_expr_opt()?;
+        // if let Some(expr) = &expr {
+        //     self.suspicious_attribute(expr, "ExprKind::Ret");
+        // }
+        let kind = ExprKind::Ret(expr);
         let expr = self.mk_expr(lo.to(self.prev_token.span), kind);
         self.maybe_recover_from_bad_qpath(expr)
     }
@@ -1881,7 +1905,11 @@ impl<'a> Parser<'a> {
         self.bump(); // `do`
         self.bump(); // `yeet`
 
-        let kind = ExprKind::Yeet(self.parse_expr_opt()?);
+        let expr = self.parse_expr_opt()?;
+        // if let Some(expr) = &expr {
+        //     self.suspicious_attribute(expr, "ExprKind::Yeet");
+        // }
+        let kind = ExprKind::Yeet(expr);
 
         let span = lo.to(self.prev_token.span);
         self.psess.gated_spans.gate(sym::yeet_expr, span);
@@ -1892,7 +1920,9 @@ impl<'a> Parser<'a> {
     /// Parse `"become" expr`, with `"become"` token already eaten.
     fn parse_expr_become(&mut self) -> PResult<'a, Box<Expr>> {
         let lo = self.prev_token.span;
-        let kind = ExprKind::Become(self.parse_expr()?);
+        let inner = self.parse_expr()?;
+        // self.suspicious_attribute(&inner, "ExprKind::Become");
+        let kind = ExprKind::Become(inner);
         let span = lo.to(self.prev_token.span);
         self.psess.gated_spans.gate(sym::explicit_tail_calls, span);
         let expr = self.mk_expr(span, kind);
@@ -1970,6 +2000,9 @@ impl<'a> Parser<'a> {
         } else {
             None
         };
+        // if let Some(expr) = &kind {
+        //     self.suspicious_attribute(expr, "ExprKind::Break");
+        // }
         let expr = self.mk_expr(lo.to(self.prev_token.span), ExprKind::Break(label, kind));
         self.maybe_recover_from_bad_qpath(expr)
     }
@@ -1994,7 +2027,11 @@ impl<'a> Parser<'a> {
     /// Parse `"yield" expr?`.
     fn parse_expr_yield(&mut self) -> PResult<'a, Box<Expr>> {
         let lo = self.prev_token.span;
-        let kind = ExprKind::Yield(YieldKind::Prefix(self.parse_expr_opt()?));
+        let expr = self.parse_expr_opt()?;
+        // if let Some(expr) = &expr {
+        //     self.suspicious_attribute(expr, "ExprKind::Yield prefix");
+        // }
+        let kind = ExprKind::Yield(YieldKind::Prefix(expr));
         let span = lo.to(self.prev_token.span);
         self.psess.gated_spans.gate(sym::yield_expr, span);
         let expr = self.mk_expr(span, kind);
@@ -2498,6 +2535,7 @@ impl<'a> Parser<'a> {
 
         let body_span = body.span;
 
+        // self.suspicious_attribute(&body, "Closure::body");
         let closure = self.mk_expr(
             lo.to(body.span),
             ExprKind::Closure(Box::new(ast::Closure {
@@ -2832,6 +2870,7 @@ impl<'a> Parser<'a> {
         }
         let expr = self.parse_expr_assoc(Bound::Excluded(prec_let_scrutinee_needs_par()))?;
         let span = lo.to(expr.span);
+        self.suspicious_attribute(&expr, "ExprKind::Let rhs");
         Ok(self.mk_expr(span, ExprKind::Let(Box::new(pat), expr, span, recovered)))
     }
 
@@ -3559,6 +3598,7 @@ impl<'a> Parser<'a> {
 
         CondChecker::new(self, LetChainsPolicy::AlwaysAllowed).visit_expr(&mut cond);
 
+        self.suspicious_attribute(&cond, "Guard::cond arm");
         let guard = Guard { cond: *cond, span_with_leading_if: leading_if_span.to(cond_span) };
         Ok(Box::new(guard))
     }
@@ -4142,6 +4182,8 @@ impl<'a> Parser<'a> {
     }
 
     fn mk_assign_op(&self, assign_op: AssignOp, lhs: Box<Expr>, rhs: Box<Expr>) -> ExprKind {
+        self.suspicious_attribute(&lhs, "ExprKind::AssignOp lhs");
+        self.suspicious_attribute(&rhs, "ExprKind::AssignOp rhs");
         ExprKind::AssignOp(assign_op, lhs, rhs)
     }
 
@@ -4155,35 +4197,48 @@ impl<'a> Parser<'a> {
             let guar = self.inclusive_range_with_incorrect_end();
             ExprKind::Err(guar)
         } else {
+            if let Some(expr) = &start {
+                self.suspicious_attribute(&expr, "ExprKind::Range lhs");
+            }
+            if let Some(expr) = &end {
+                self.suspicious_attribute(&expr, "ExprKind::Range rhs");
+            }
             ExprKind::Range(start, end, limits)
         }
     }
 
     fn mk_unary(&self, unop: UnOp, expr: Box<Expr>) -> ExprKind {
+        self.suspicious_attribute(&expr, "ExprKind::Unary");
         ExprKind::Unary(unop, expr)
     }
 
     fn mk_binary(&self, binop: BinOp, lhs: Box<Expr>, rhs: Box<Expr>) -> ExprKind {
+        self.suspicious_attribute(&lhs, "ExprKind::Binary lhs");
+        self.suspicious_attribute(&rhs, "ExprKind::Binary rhs");
         ExprKind::Binary(binop, lhs, rhs)
     }
 
     fn mk_index(&self, expr: Box<Expr>, idx: Box<Expr>, brackets_span: Span) -> ExprKind {
+        // self.suspicious_attribute(&expr, "ExprKind::Index");
         ExprKind::Index(expr, idx, brackets_span)
     }
 
     fn mk_call(&self, f: Box<Expr>, args: ThinVec<Box<Expr>>) -> ExprKind {
+        // self.suspicious_attribute(&f, "ExprKind::Call");
         ExprKind::Call(f, args)
     }
 
     fn mk_await_expr(&mut self, self_arg: Box<Expr>, lo: Span) -> Box<Expr> {
         let span = lo.to(self.prev_token.span);
+        // self.suspicious_attribute(&self_arg, "ExprKind::Await");
         let await_expr = self.mk_expr(span, ExprKind::Await(self_arg, self.prev_token.span));
         self.recover_from_await_method_call();
         await_expr
     }
 
     fn mk_use_expr(&mut self, self_arg: Box<Expr>, lo: Span) -> Box<Expr> {
         let span = lo.to(self.prev_token.span);
+        // self.suspicious_attribute(&self_arg, "ExprKind::Use");
         let use_expr = self.mk_expr(span, ExprKind::Use(self_arg, self.prev_token.span));
         self.recover_from_use();
         use_expr

compiler/rustc_parse/src/parser/pat.rs

@@ -116,6 +116,7 @@ impl<'a> Parser<'a> {
                 let leading_if_span = self.prev_token.span;
                 let cond = self.parse_expr()?;
                 let cond_span = cond.span;
+                self.suspicious_attribute(&cond, "Guard::cond pat");
                 Box::new(Guard { cond: *cond, span_with_leading_if: leading_if_span.to(cond_span) })
             };
 
@@ -799,6 +800,7 @@ impl<'a> Parser<'a> {
             if let Some(re) = self.parse_range_end() {
                 self.parse_pat_range_begin_with(const_expr, re)?
             } else {
+                self.suspicious_attribute(&const_expr, "PatKind::Expr const_block");
                 PatKind::Expr(const_expr)
             }
         } else if self.is_builtin() {
@@ -1373,6 +1375,10 @@ impl<'a> Parser<'a> {
             }
             None
         };
+        self.suspicious_attribute(&begin, "PatKind::Range lhs");
+        if let Some(expr) = &end {
+            self.suspicious_attribute(&expr, "PatKind::Range rhs");
+        }
         Ok(PatKind::Range(Some(begin), end, re))
     }
 
@@ -1414,6 +1420,7 @@ impl<'a> Parser<'a> {
             *syn = RangeSyntax::DotDotEq;
             self.dcx().emit_err(DotDotDotRangeToPatternNotAllowed { span: re.span });
         }
+        self.suspicious_attribute(&end, "PatKind::Range end");
         Ok(PatKind::Range(None, Some(end), re))
     }
 

compiler/rustc_parse/src/parser/path.rs

@@ -873,6 +873,7 @@ impl<'a> Parser<'a> {
         } else {
             self.parse_unambiguous_unbraced_const_arg()?
         };
+        self.suspicious_attribute(&value, "AnonConst::value const_arg");
         Ok(AnonConst { id: ast::DUMMY_NODE_ID, value })
     }
 

expr-attr-2.md

@@ -0,0 +1,56 @@
+Regarding https://github.com/rust-lang/rust/pull/159581#issuecomment-5095419394.
+
+### Suspicious contexts (single -> list)
+
+In all these cases if `$expr` has a top level attribute inside it, it will migrate from a single to list context if the parentheses from the macro variable are not correctly preserved in AST.
+
+```rust
+// method call arguments
+r.method($expr, ...)
+
+// array element or size
+[$expr, 11, 12]
+
+// function call argument
+func($expr, 11, 12)
+
+// tuple element
+($expr, 11, 12)
+
+// const generic arguments (if supported at all)
+foo::<$expr>()
+```
+
+### Suspicious contexts (expr -> stmt)
+
+In all these cases if `$expr` has a top level attribute inside it, it will migrate from a single expression to statement context if the parentheses from the macro variable are not correctly preserved in AST.
+
+```rust
+// $expr is:
+
+// postfix await, yield or use
+#[attr] val.await
+#[attr] val.yield
+#[attr] val.use
+
+// field or method access
+#[attr] val.field
+#[attr] val.method()
+
+// function call or indexing
+#[attr] val()
+#[attr] val[]
+
+// expression
+#[attr] val?
+
+// path
+#[attr] path
+```
+
+If an attribute starts a statement as a part of some larger expression (e.g. a binary operator), it will already be reported as an error by https://github.com/rust-lang/rust/pull/160235#issuecomment-5159173426.
+
+### Conclusion
+
+These examples are only ambiguous if any of the involved attributes are active (including `cfg`), except that `cfg_attr` is ok (because we know what it does, and it does not depend on context).
+So we don't need to gate any of this during parsing, and can continue using expansion-time gating.