diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 34044e72ab92b..462711d7559b9 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -139,6 +139,14 @@ impl<'a> Parser<'a> { self.parse_expr_assoc_rest_with(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}")); + } + } + } + /// 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. @@ -290,7 +298,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) @@ -674,6 +686,7 @@ impl<'a> Parser<'a> { expr_kind: fn(Box, Box) -> ExprKind, ) -> PResult<'a, Box> { let mk_expr = |this: &mut Self, lhs: Box, rhs: Box| { + this.suspicious_attribute(&lhs, "ExprKind::Cast"); this.mk_expr(this.mk_expr_sp(&lhs, lhs_span, op_span, rhs.span), expr_kind(lhs, rhs)) }; @@ -864,6 +877,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))) } @@ -919,6 +933,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; } @@ -1264,6 +1279,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))) } @@ -1425,6 +1441,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))) ); @@ -1440,6 +1457,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 { @@ -1459,6 +1477,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))) } } @@ -1905,7 +1924,11 @@ impl<'a> Parser<'a> { /// Parse `"return" expr?`. fn parse_expr_return(&mut self) -> PResult<'a, Box> { 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) } @@ -1917,7 +1940,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); @@ -1928,7 +1955,9 @@ impl<'a> Parser<'a> { /// Parse `"become" expr`, with `"become"` token already eaten. fn parse_expr_become(&mut self) -> PResult<'a, Box> { 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); @@ -2006,6 +2035,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) } @@ -2030,7 +2062,11 @@ impl<'a> Parser<'a> { /// Parse `"yield" expr?`. fn parse_expr_yield(&mut self) -> PResult<'a, Box> { 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); @@ -2537,6 +2573,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 { @@ -2863,6 +2900,7 @@ impl<'a> Parser<'a> { let (expr, _) = self.parse_expr_assoc_with(Bound::Excluded(prec_let_scrutinee_needs_par()), attrs)?; 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))) } @@ -3594,6 +3632,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)) } @@ -4181,6 +4220,8 @@ impl<'a> Parser<'a> { } fn mk_assign_op(&self, assign_op: AssignOp, lhs: Box, rhs: Box) -> ExprKind { + self.suspicious_attribute(&lhs, "ExprKind::AssignOp lhs"); + self.suspicious_attribute(&rhs, "ExprKind::AssignOp rhs"); ExprKind::AssignOp(assign_op, lhs, rhs) } @@ -4194,28 +4235,40 @@ 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) -> ExprKind { + self.suspicious_attribute(&expr, "ExprKind::Unary"); ExprKind::Unary(unop, expr) } fn mk_binary(&self, binop: BinOp, lhs: Box, rhs: Box) -> 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, idx: Box, brackets_span: Span) -> ExprKind { + self.suspicious_attribute(&expr, "ExprKind::Index"); ExprKind::Index(expr, idx, brackets_span) } fn mk_call(&self, f: Box, args: ThinVec>) -> ExprKind { + self.suspicious_attribute(&f, "ExprKind::Call"); ExprKind::Call(f, args) } fn mk_await_expr(&mut self, self_arg: Box, lo: Span) -> Box { 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 @@ -4223,6 +4276,7 @@ impl<'a> Parser<'a> { fn mk_use_expr(&mut self, self_arg: Box, lo: Span) -> Box { 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 diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index 74b2194cc97fa..4ed9349d1566d 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -117,6 +117,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) }) }; @@ -798,6 +799,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() { @@ -1216,6 +1218,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)) } @@ -1257,6 +1263,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)) } diff --git a/compiler/rustc_parse/src/parser/path.rs b/compiler/rustc_parse/src/parser/path.rs index 066d4402d5fde..8b216df8316de 100644 --- a/compiler/rustc_parse/src/parser/path.rs +++ b/compiler/rustc_parse/src/parser/path.rs @@ -880,6 +880,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 }) } diff --git a/expr-attr-2.md b/expr-attr-2.md new file mode 100644 index 0000000000000..01e51a9a72d8a --- /dev/null +++ b/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. diff --git a/expr-attr.md b/expr-attr.md new file mode 100644 index 0000000000000..92a19af493116 --- /dev/null +++ b/expr-attr.md @@ -0,0 +1,195 @@ +Attributes in expressions, purely from syntactic point of view, at parsing time. +I.e. no distinction between active or inert, or built-in or custom attributes. +And without considering https://github.com/rust-lang/rust/pull/159581#issuecomment-5095419394. + +### Not syntactically suspicious (top level expressions): + +Definitely can stabilize after accounting for macro attributes and https://github.com/rust-lang/rust/pull/159581#issuecomment-5095419394. + +```rust +// let initializer +let x = #[attr] 10; +let x = #[attr] 10 else { ... }; + +// const or static item initializer +const C: u8 = #[attr] 10; +static C: u8 = #[attr] 10; + +// enum discriminant value +enum E { V = #[attr] 10 } + +// field default +struct S { field: u8 = #[attr] 10 } + +// key value attribute +#[key = #[attr] value] + +// `match` arm +match 10 { + 11 => #[attr] 12, +} + +// struct literals fields +let x = Struct { field: #[attr] 10, ... } + +// struct literals rest +let x = Struct { field: 10, ..#[attr] base } + +// for loop iterator +for x in #[attr] 10 { ... } + +// if condition +if #[attr] 10 { ... } + +// while condition +while #[attr] 10 { ... } + +// match scrutinee +match #[attr] 10 { ... } + +// method call arguments +r.method(#[attr] 10, ...) + +// array element or size +[#[attr] 10, 11, 12] +[#[attr] 10; 11] +[10; #[attr] 11] +[u8; #[attr] 11] + +// function call argument +func(#[attr] 10, 11, 12) + +// tuple element +(#[attr] 10, 11, 12) + +// move expression +move(#[attr] 10) + +// indexing argument +array[#[attr] 10] + +// parentheses +(#[attr] 10) + +// const generic defaults +fn foo() {} + +// const parameter constraint +foo::() + +// const generic arguments (if supported at all) +foo::<#[attr] 10>() + +// type ascription +builtin # type_ascribe(#[attr] 10, u8) + +// unsafe binder casts +builtin # wrap_binder(#[attr] 10, u8) + +// various inline asm operands +asm!("code", in("r") #[attr] x) + +// some syntax for contracts, I didn't look closely +``` + +### Semi-suspicious (top level expressions): + +I don't see any actual issues, just would be interesting how often these will occur in the crater run. + +Can stabilize (after accounting for macro attributes and https://github.com/rust-lang/rust/pull/159581#issuecomment-5095419394) or not stabilize. + +```rust +// closure body +let x = || #[attr] 10; + +// break, return, yield, become, and yeet expressions +break #[attr] 10 +return #[attr] 10 +yield #[attr] 10 +become #[attr] 10 +do yeet #[attr] 10 + +// match or pattern guard conditions +pat if #[attr] true + +// `expr` matcher as an await or yield receiver +$expr.await // `$expr` is `#[attr] 10` +$expr.yield // `$expr` is `#[attr] 10` + +// `expr` matcher as a use receiver +$expr.use // `$expr` is `#[attr] 10` + +// `expr` matcher as a field receiver +$expr.field // `$expr` is `#[attr] 10` + +// `expr` matcher as a method receiver +$expr.method() // `$expr` is `#[attr] 10` + +// `expr` matcher as a callable in a function call +$expr() // `$expr` is `#[attr] 10` + +// `expr` matcher as an indexable expression +$expr[] // `$expr` is `#[attr] 10` + +// `expr` matcher as a try expression +$expr? // `$expr` is `#[attr] 10` +``` + +### Semi-suspicious (non-top level expressions): + +I don't see any actual issues, just would be interesting how often these will occur in the crater run. + +Can stabilize (after accounting for macro attributes and https://github.com/rust-lang/rust/pull/159581#issuecomment-5095419394) or not stabilize. + +```rust +// binary operator rhs +10 + #[attr] 11 + +// assignment rhs +lhs = #[attr] rhs +lhs += #[attr] rhs + +// range rhs +10 .. #[attr] 11 + +// let expressions in if/while +if let x = #[attr] 10 && x == 11 { ... } + +// prefix unary operator +- #[attr] 10 + +// reference operator +& #[attr] 10 +& raw mut #[attr] 10 +``` + +### Suspicious (top level expressions): + +Cannot stabilize, this is a long stanging issue with too permissive pattern parsing. + +```rust +// `expr` matcher as a pattern +match 10 { $expr => {} } // `$expr` is `#[attr] 10` + +// `expr` matcher in a range pattern +match 10 { $expr .. $expr => {} } // `$expr` is `#[attr] 10` +``` + +### Suspicious (non-top level expressions): + +Cannot stabilize. + +```rust +// binary operator lhs +#[attr] 10 + 11 + +// assignment lhs +#[attr] lhs = rhs +#[attr] lhs += rhs + +// range lhs +#[attr] 10 .. 11 + +// cast expressions +#[attr] 10 as u8 +```