From edf65e735cd871d01149131f5d050293a9f1037c Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Wed, 12 Mar 2025 15:59:28 -0700 Subject: [PATCH 01/16] Add support for postfix yield expressions We had a discussion[1] today about whether postfix yield would make sense. It's easy enough to support both in the parser, so we might as well have both and see how people use it while the feature is experimental. [1]: https://rust-lang.zulipchat.com/#narrow/channel/481571-t-lang.2Fgen/topic/postfix-yield/with/505231568 --- compiler/rustc_parse/src/parser/expr.rs | 7 +++++ tests/ui/coroutine/postfix-yield.rs | 34 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 tests/ui/coroutine/postfix-yield.rs diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 9b2d562a69e..1df6283af26 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1310,6 +1310,13 @@ impl<'a> Parser<'a> { return self.parse_match_block(lo, match_span, self_arg, MatchKind::Postfix); } + // Post-fix yield + if self.eat_keyword(exp!(Yield)) { + let yield_span = self.prev_token.span; + self.psess.gated_spans.gate(sym::yield_expr, yield_span); + return Ok(self.mk_expr(yield_span, ExprKind::Yield(Some(self_arg)))); + } + let fn_span_lo = self.token.span; let mut seg = self.parse_path_segment(PathStyle::Expr, None)?; self.check_trailing_angle_brackets(&seg, &[exp!(OpenParen)]); diff --git a/tests/ui/coroutine/postfix-yield.rs b/tests/ui/coroutine/postfix-yield.rs new file mode 100644 index 00000000000..77b1c2a19d0 --- /dev/null +++ b/tests/ui/coroutine/postfix-yield.rs @@ -0,0 +1,34 @@ +// This demonstrates a proposed alternate or additional option of having yield in postfix position. + +//@ run-pass +//@ edition: 2024 + +#![feature(gen_blocks, coroutines, coroutine_trait, yield_expr)] + +use std::ops::{Coroutine, CoroutineState}; +use std::pin::pin; + +fn main() { + // generators (i.e. yield doesn't return anything useful) + let mut gn = gen { + yield 1; + 2.yield; + }; + + assert_eq!(gn.next(), Some(1)); + assert_eq!(gn.next(), Some(2)); + assert_eq!(gn.next(), None); + + //coroutines (i.e. yield returns something useful) + let mut coro = pin!( + #[coroutine] + |_: i32| { + let x = yield 1; + yield x + 2; + } + ); + + assert_eq!(coro.as_mut().resume(0), CoroutineState::Yielded(1)); + assert_eq!(coro.as_mut().resume(2), CoroutineState::Yielded(4)); + assert_eq!(coro.as_mut().resume(3), CoroutineState::Complete(())); +} From 1c0916a2b3cd6c595e1c7b69a31d507f7619bb67 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Wed, 12 Mar 2025 16:27:52 -0700 Subject: [PATCH 02/16] Preserve yield position during pretty printing --- compiler/rustc_ast/src/ast.rs | 11 ++++++++++- compiler/rustc_ast/src/mut_visit.rs | 2 +- compiler/rustc_ast/src/util/classify.rs | 4 ++-- compiler/rustc_ast/src/visit.rs | 2 +- compiler/rustc_ast_lowering/src/expr.rs | 2 +- compiler/rustc_ast_lowering/src/format.rs | 2 +- .../rustc_ast_pretty/src/pprust/state/expr.rs | 14 ++++++++++++-- .../rustc_builtin_macros/src/assert/context.rs | 2 +- compiler/rustc_parse/src/parser/expr.rs | 9 ++++++--- src/tools/rustfmt/src/utils.rs | 4 ++-- tests/pretty/postfix-yield.rs | 15 +++++++++++++++ tests/ui/coroutine/postfix-yield.rs | 6 +++--- 12 files changed, 55 insertions(+), 18 deletions(-) create mode 100644 tests/pretty/postfix-yield.rs diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 1b831c454e6..9dcdd868343 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1657,7 +1657,7 @@ pub enum ExprKind { Try(P), /// A `yield`, with an optional value to be yielded. - Yield(Option>), + Yield(Option>, YieldKind), /// A `do yeet` (aka `throw`/`fail`/`bail`/`raise`/whatever), /// with an optional value to be returned. @@ -1903,6 +1903,15 @@ pub enum MatchKind { Postfix, } +/// The kind of yield expression +#[derive(Clone, Copy, Encodable, Decodable, Debug, PartialEq)] +pub enum YieldKind { + /// yield expr { ... } + Prefix, + /// expr.yield { ... } + Postfix, +} + /// A literal in a meta item. #[derive(Clone, Encodable, Decodable, Debug, HashStable_Generic)] pub struct MetaItemLit { diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 4a1636e6aec..5a9df6ffadf 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1813,7 +1813,7 @@ pub fn walk_expr(vis: &mut T, Expr { kind, id, span, attrs, token ExprKind::Paren(expr) => { vis.visit_expr(expr); } - ExprKind::Yield(expr) => { + ExprKind::Yield(expr, _) => { visit_opt(expr, |expr| vis.visit_expr(expr)); } ExprKind::Try(expr) => vis.visit_expr(expr), diff --git a/compiler/rustc_ast/src/util/classify.rs b/compiler/rustc_ast/src/util/classify.rs index e43d78f6e72..116847af9f4 100644 --- a/compiler/rustc_ast/src/util/classify.rs +++ b/compiler/rustc_ast/src/util/classify.rs @@ -182,7 +182,7 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option> { | Range(_, Some(e), _) | Ret(Some(e)) | Unary(_, e) - | Yield(Some(e)) + | Yield(Some(e), _) | Yeet(Some(e)) | Become(e) => { expr = e; @@ -217,7 +217,7 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option> { Break(_, None) | Range(_, None, _) | Ret(None) - | Yield(None) + | Yield(None, _) | Array(_) | Call(_, _) | MethodCall(_) diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index cfcb0e23cb5..a72e834e17a 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -1269,7 +1269,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) -> V try_visit!(visitor.visit_ty(container)); walk_list!(visitor, visit_ident, fields.iter()); } - ExprKind::Yield(optional_expression) => { + ExprKind::Yield(optional_expression, _) => { visit_opt!(visitor, visit_expr, optional_expression); } ExprKind::Try(subexpression) => try_visit!(visitor.visit_expr(subexpression)), diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 5bb6704dde4..19a08f92ce7 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -351,7 +351,7 @@ impl<'hir> LoweringContext<'_, 'hir> { rest, ) } - ExprKind::Yield(opt_expr) => self.lower_expr_yield(e.span, opt_expr.as_deref()), + ExprKind::Yield(opt_expr, _) => self.lower_expr_yield(e.span, opt_expr.as_deref()), ExprKind::Err(guar) => hir::ExprKind::Err(*guar), ExprKind::UnsafeBinderCast(kind, expr, ty) => hir::ExprKind::UnsafeBinderCast( diff --git a/compiler/rustc_ast_lowering/src/format.rs b/compiler/rustc_ast_lowering/src/format.rs index faa47274f96..2bbf957feeb 100644 --- a/compiler/rustc_ast_lowering/src/format.rs +++ b/compiler/rustc_ast_lowering/src/format.rs @@ -642,7 +642,7 @@ fn may_contain_yield_point(e: &ast::Expr) -> bool { type Result = ControlFlow<()>; fn visit_expr(&mut self, e: &ast::Expr) -> ControlFlow<()> { - if let ast::ExprKind::Await(_, _) | ast::ExprKind::Yield(_) = e.kind { + if let ast::ExprKind::Await(_, _) | ast::ExprKind::Yield(_, _) = e.kind { ControlFlow::Break(()) } else { visit::walk_expr(self, e) diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs index e3c41f117ab..9e53ed41c46 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs @@ -8,7 +8,7 @@ use rustc_ast::util::literal::escape_byte_str_symbol; use rustc_ast::util::parser::{self, ExprPrecedence, Fixity}; use rustc_ast::{ self as ast, BlockCheckMode, FormatAlignment, FormatArgPosition, FormatArgsPiece, FormatCount, - FormatDebugHex, FormatSign, FormatTrait, token, + FormatDebugHex, FormatSign, FormatTrait, YieldKind, token, }; use crate::pp::Breaks::Inconsistent; @@ -761,7 +761,7 @@ impl<'a> State<'a> { self.print_expr(e, FixupContext::default()); self.pclose(); } - ast::ExprKind::Yield(e) => { + ast::ExprKind::Yield(e, YieldKind::Prefix) => { self.word("yield"); if let Some(expr) = e { @@ -773,6 +773,16 @@ impl<'a> State<'a> { ); } } + ast::ExprKind::Yield(e, YieldKind::Postfix) => { + // it's not possible to have a postfix yield with no expression. + let e = e.as_ref().unwrap(); + self.print_expr_cond_paren( + e, + e.precedence() < ExprPrecedence::Unambiguous, + fixup.leftmost_subexpression_with_dot(), + ); + self.word(".yield"); + } ast::ExprKind::Try(e) => { self.print_expr_cond_paren( e, diff --git a/compiler/rustc_builtin_macros/src/assert/context.rs b/compiler/rustc_builtin_macros/src/assert/context.rs index a949ab94f3a..54c3cebedfc 100644 --- a/compiler/rustc_builtin_macros/src/assert/context.rs +++ b/compiler/rustc_builtin_macros/src/assert/context.rs @@ -323,7 +323,7 @@ impl<'cx, 'a> Context<'cx, 'a> { | ExprKind::While(_, _, _) | ExprKind::Yeet(_) | ExprKind::Become(_) - | ExprKind::Yield(_) + | ExprKind::Yield(_, _) | ExprKind::UnsafeBinderCast(..) => {} } } diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 1df6283af26..cb04bbc240e 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -17,6 +17,7 @@ use rustc_ast::{ self as ast, AnonConst, Arm, AttrStyle, AttrVec, BinOp, BinOpKind, BlockCheckMode, CaptureBy, ClosureBinder, DUMMY_NODE_ID, Expr, ExprField, ExprKind, FnDecl, FnRetTy, Label, MacCall, MetaItemLit, Movability, Param, RangeLimits, StmtKind, Ty, TyKind, UnOp, UnsafeBinderCastKind, + YieldKind, }; use rustc_ast_pretty::pprust; use rustc_data_structures::stack::ensure_sufficient_stack; @@ -1314,7 +1315,9 @@ 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); - return Ok(self.mk_expr(yield_span, ExprKind::Yield(Some(self_arg)))); + return Ok( + self.mk_expr(yield_span, ExprKind::Yield(Some(self_arg), YieldKind::Postfix)) + ); } let fn_span_lo = self.token.span; @@ -1891,7 +1894,7 @@ impl<'a> Parser<'a> { /// Parse `"yield" expr?`. fn parse_expr_yield(&mut self) -> PResult<'a, P> { let lo = self.prev_token.span; - let kind = ExprKind::Yield(self.parse_expr_opt()?); + let kind = ExprKind::Yield(self.parse_expr_opt()?, YieldKind::Prefix); let span = lo.to(self.prev_token.span); self.psess.gated_spans.gate(sym::yield_expr, span); let expr = self.mk_expr(span, kind); @@ -4045,7 +4048,7 @@ impl MutVisitor for CondChecker<'_> { | ExprKind::MacCall(_) | ExprKind::Struct(_) | ExprKind::Repeat(_, _) - | ExprKind::Yield(_) + | ExprKind::Yield(_, _) | ExprKind::Yeet(_) | ExprKind::Become(_) | ExprKind::IncludedBytes(_) diff --git a/src/tools/rustfmt/src/utils.rs b/src/tools/rustfmt/src/utils.rs index fe716c18638..bee39153229 100644 --- a/src/tools/rustfmt/src/utils.rs +++ b/src/tools/rustfmt/src/utils.rs @@ -485,7 +485,7 @@ pub(crate) fn is_block_expr(context: &RewriteContext<'_>, expr: &ast::Expr, repr | ast::ExprKind::Index(_, ref expr, _) | ast::ExprKind::Unary(_, ref expr) | ast::ExprKind::Try(ref expr) - | ast::ExprKind::Yield(Some(ref expr)) => is_block_expr(context, expr, repr), + | ast::ExprKind::Yield(Some(ref expr), _) => is_block_expr(context, expr, repr), ast::ExprKind::Closure(ref closure) => is_block_expr(context, &closure.body, repr), // This can only be a string lit ast::ExprKind::Lit(_) => { @@ -515,7 +515,7 @@ pub(crate) fn is_block_expr(context: &RewriteContext<'_>, expr: &ast::Expr, repr | ast::ExprKind::Tup(..) | ast::ExprKind::Use(..) | ast::ExprKind::Type(..) - | ast::ExprKind::Yield(None) + | ast::ExprKind::Yield(None, _) | ast::ExprKind::Underscore => false, } } diff --git a/tests/pretty/postfix-yield.rs b/tests/pretty/postfix-yield.rs new file mode 100644 index 00000000000..f76e8142ae8 --- /dev/null +++ b/tests/pretty/postfix-yield.rs @@ -0,0 +1,15 @@ +// This demonstrates a proposed alternate or additional option of having yield in postfix position. +//@ edition: 2024 +//@ pp-exact + +#![feature(gen_blocks, coroutines, coroutine_trait, yield_expr)] + +use std::ops::{Coroutine, CoroutineState}; +use std::pin::pin; + +fn main() { + let mut gn = gen { yield 1; 2.yield; (1 + 2).yield; }; + + let mut coro = + pin!(#[coroutine] |_: i32| { let x = 1.yield; (x + 2).yield; }); +} diff --git a/tests/ui/coroutine/postfix-yield.rs b/tests/ui/coroutine/postfix-yield.rs index 77b1c2a19d0..ff843138c8c 100644 --- a/tests/ui/coroutine/postfix-yield.rs +++ b/tests/ui/coroutine/postfix-yield.rs @@ -9,7 +9,7 @@ use std::ops::{Coroutine, CoroutineState}; use std::pin::pin; fn main() { - // generators (i.e. yield doesn't return anything useful) + // generators (i.e. yield doesn't return anything useful) let mut gn = gen { yield 1; 2.yield; @@ -23,8 +23,8 @@ fn main() { let mut coro = pin!( #[coroutine] |_: i32| { - let x = yield 1; - yield x + 2; + let x = 1.yield; + (x + 2).yield; } ); From 635eae2d4fc724cb53a3a45975bc18779a3293b1 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Thu, 13 Mar 2025 14:36:02 -0700 Subject: [PATCH 03/16] Teach rustfmt to handle postfix yield --- .../clippy/clippy_utils/src/ast_utils/mod.rs | 5 +++-- src/tools/rustfmt/src/expr.rs | 5 +++-- src/tools/rustfmt/tests/source/postfix-yield.rs | 15 +++++++++++++++ src/tools/rustfmt/tests/target/postfix-yield.rs | 12 ++++++++++++ 4 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 src/tools/rustfmt/tests/source/postfix-yield.rs create mode 100644 src/tools/rustfmt/tests/target/postfix-yield.rs diff --git a/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs b/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs index 707312a97f3..deda6030831 100644 --- a/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs +++ b/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs @@ -201,7 +201,8 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool { (Loop(lt, ll, _), Loop(rt, rl, _)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_block(lt, rt), (Block(lb, ll), Block(rb, rl)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_block(lb, rb), (TryBlock(l), TryBlock(r)) => eq_block(l, r), - (Yield(l), Yield(r)) | (Ret(l), Ret(r)) => eq_expr_opt(l.as_ref(), r.as_ref()), + (Yield(l, lk), Yield(r, rk)) => eq_expr_opt(l.as_ref(), r.as_ref()) && lk == rk, + (Ret(l), Ret(r)) => eq_expr_opt(l.as_ref(), r.as_ref()), (Break(ll, le), Break(rl, re)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_expr_opt(le.as_ref(), re.as_ref()), (Continue(ll), Continue(rl)) => eq_label(ll.as_ref(), rl.as_ref()), (Assign(l1, l2, _), Assign(r1, r2, _)) | (Index(l1, l2, _), Index(r1, r2, _)) => { @@ -688,7 +689,7 @@ pub fn eq_generics(l: &Generics, r: &Generics) -> bool { pub fn eq_where_predicate(l: &WherePredicate, r: &WherePredicate) -> bool { use WherePredicateKind::*; - over(&l.attrs, &r.attrs, eq_attr) + over(&l.attrs, &r.attrs, eq_attr) && match (&l.kind, &r.kind) { (BoundPredicate(l), BoundPredicate(r)) => { over(&l.bound_generic_params, &r.bound_generic_params, |l, r| { diff --git a/src/tools/rustfmt/src/expr.rs b/src/tools/rustfmt/src/expr.rs index eff2d2e3ff4..92c1ffa6076 100644 --- a/src/tools/rustfmt/src/expr.rs +++ b/src/tools/rustfmt/src/expr.rs @@ -221,7 +221,7 @@ pub(crate) fn format_expr( Ok(format!("break{id_str}")) } } - ast::ExprKind::Yield(ref opt_expr) => { + ast::ExprKind::Yield(ref opt_expr, ast::YieldKind::Prefix) => { if let Some(ref expr) = *opt_expr { rewrite_unary_prefix(context, "yield ", &**expr, shape) } else { @@ -243,7 +243,8 @@ pub(crate) fn format_expr( ast::ExprKind::Try(..) | ast::ExprKind::Field(..) | ast::ExprKind::MethodCall(..) - | ast::ExprKind::Await(_, _) => rewrite_chain(expr, context, shape), + | ast::ExprKind::Await(_, _) + | ast::ExprKind::Yield(_, ast::YieldKind::Postfix) => rewrite_chain(expr, context, shape), ast::ExprKind::MacCall(ref mac) => { rewrite_macro(mac, None, context, shape, MacroPosition::Expression).or_else(|_| { wrap_str( diff --git a/src/tools/rustfmt/tests/source/postfix-yield.rs b/src/tools/rustfmt/tests/source/postfix-yield.rs new file mode 100644 index 00000000000..8a8958f3ad4 --- /dev/null +++ b/src/tools/rustfmt/tests/source/postfix-yield.rs @@ -0,0 +1,15 @@ +// This demonstrates a proposed alternate or additional option of having yield in postfix position. +//@ edition: 2024 + +#![feature(gen_blocks, coroutines, coroutine_trait, yield_expr)] + +use std::ops::{Coroutine, CoroutineState}; +use std::pin::pin; + +fn main() { + let mut coro = + pin!(#[coroutine] |_: i32| { let x = 1.yield; + + + (x + 2).yield; }); +} diff --git a/src/tools/rustfmt/tests/target/postfix-yield.rs b/src/tools/rustfmt/tests/target/postfix-yield.rs new file mode 100644 index 00000000000..7e94e1e095a --- /dev/null +++ b/src/tools/rustfmt/tests/target/postfix-yield.rs @@ -0,0 +1,12 @@ +// This demonstrates a proposed alternate or additional option of having yield in postfix position. +//@ edition: 2024 + +#![feature(gen_blocks, coroutines, coroutine_trait, yield_expr)] + +use std::ops::{Coroutine, CoroutineState}; +use std::pin::pin; + +fn main() { + let mut coro = + pin!(#[coroutine] |_: i32| { let x = 1.yield; (x + 2).yield; }); +} From c5093ac1224fe9eeff5c5694f1c3ff643005d7d4 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Thu, 13 Mar 2025 16:14:31 -0700 Subject: [PATCH 04/16] Fix clippy --- .../clippy/clippy_lints/src/suspicious_operation_groupings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs b/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs index 0d809c17989..206912d8de4 100644 --- a/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs +++ b/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs @@ -528,7 +528,7 @@ fn ident_difference_expr_with_base_location( &strip_non_ident_wrappers(left).kind, &strip_non_ident_wrappers(right).kind, ) { - (Yield(_), Yield(_)) + (Yield(_, _), Yield(_, _)) | (Try(_), Try(_)) | (Paren(_), Paren(_)) | (Repeat(_, _), Repeat(_, _)) From 9b0e7f62644b629c30b4157ad854296eca36ecf0 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Mon, 17 Mar 2025 17:32:11 -0700 Subject: [PATCH 05/16] Teach rustfmt to handle postfix yield This involved fixing the span when parsing .yield --- compiler/rustc_parse/src/parser/expr.rs | 5 ++--- src/tools/rustfmt/src/chains.rs | 10 +++++++++- src/tools/rustfmt/src/utils.rs | 8 +++++--- src/tools/rustfmt/tests/source/postfix-yield.rs | 15 --------------- src/tools/rustfmt/tests/target/postfix-yield.rs | 9 +++++++-- 5 files changed, 23 insertions(+), 24 deletions(-) delete mode 100644 src/tools/rustfmt/tests/source/postfix-yield.rs diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index cb04bbc240e..28d100074f3 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1315,9 +1315,8 @@ 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); - return Ok( - self.mk_expr(yield_span, ExprKind::Yield(Some(self_arg), YieldKind::Postfix)) - ); + return Ok(self + .mk_expr(lo.to(yield_span), ExprKind::Yield(Some(self_arg), YieldKind::Postfix))); } let fn_span_lo = self.token.span; diff --git a/src/tools/rustfmt/src/chains.rs b/src/tools/rustfmt/src/chains.rs index fd2ef9cb1db..fabb4400553 100644 --- a/src/tools/rustfmt/src/chains.rs +++ b/src/tools/rustfmt/src/chains.rs @@ -192,6 +192,7 @@ enum ChainItemKind { StructField(symbol::Ident), TupleField(symbol::Ident, bool), Await, + Yield, Comment(String, CommentPosition), } @@ -203,6 +204,7 @@ impl ChainItemKind { | ChainItemKind::StructField(..) | ChainItemKind::TupleField(..) | ChainItemKind::Await + | ChainItemKind::Yield | ChainItemKind::Comment(..) => false, } } @@ -257,6 +259,10 @@ impl ChainItemKind { let span = mk_sp(nested.span.hi(), expr.span.hi()); (ChainItemKind::Await, span) } + ast::ExprKind::Yield(Some(ref nested), ast::YieldKind::Postfix) => { + let span = mk_sp(nested.span.hi(), expr.span.hi()); + (ChainItemKind::Yield, span) + } _ => { return ( ChainItemKind::Parent { @@ -306,6 +312,7 @@ impl Rewrite for ChainItem { rewrite_ident(context, ident) ), ChainItemKind::Await => ".await".to_owned(), + ChainItemKind::Yield => ".yield".to_owned(), ChainItemKind::Comment(ref comment, _) => { rewrite_comment(comment, false, shape, context.config)? } @@ -508,7 +515,8 @@ impl Chain { }), ast::ExprKind::Field(ref subexpr, _) | ast::ExprKind::Try(ref subexpr) - | ast::ExprKind::Await(ref subexpr, _) => Some(SubExpr { + | ast::ExprKind::Await(ref subexpr, _) + | ast::ExprKind::Yield(Some(ref subexpr), ast::YieldKind::Postfix) => Some(SubExpr { expr: Self::convert_try(subexpr, context), is_method_call_receiver: false, }), diff --git a/src/tools/rustfmt/src/utils.rs b/src/tools/rustfmt/src/utils.rs index bee39153229..1811752c3c4 100644 --- a/src/tools/rustfmt/src/utils.rs +++ b/src/tools/rustfmt/src/utils.rs @@ -4,7 +4,7 @@ use rustc_ast::ast::{ self, Attribute, MetaItem, MetaItemInner, MetaItemKind, NodeId, Path, Visibility, VisibilityKind, }; -use rustc_ast::ptr; +use rustc_ast::{YieldKind, ptr}; use rustc_ast_pretty::pprust; use rustc_span::{BytePos, LocalExpnId, Span, Symbol, SyntaxContext, sym, symbol}; use unicode_width::UnicodeWidthStr; @@ -485,7 +485,9 @@ pub(crate) fn is_block_expr(context: &RewriteContext<'_>, expr: &ast::Expr, repr | ast::ExprKind::Index(_, ref expr, _) | ast::ExprKind::Unary(_, ref expr) | ast::ExprKind::Try(ref expr) - | ast::ExprKind::Yield(Some(ref expr), _) => is_block_expr(context, expr, repr), + | ast::ExprKind::Yield(Some(ref expr), YieldKind::Prefix) => { + is_block_expr(context, expr, repr) + } ast::ExprKind::Closure(ref closure) => is_block_expr(context, &closure.body, repr), // This can only be a string lit ast::ExprKind::Lit(_) => { @@ -515,7 +517,7 @@ pub(crate) fn is_block_expr(context: &RewriteContext<'_>, expr: &ast::Expr, repr | ast::ExprKind::Tup(..) | ast::ExprKind::Use(..) | ast::ExprKind::Type(..) - | ast::ExprKind::Yield(None, _) + | ast::ExprKind::Yield(_, _) | ast::ExprKind::Underscore => false, } } diff --git a/src/tools/rustfmt/tests/source/postfix-yield.rs b/src/tools/rustfmt/tests/source/postfix-yield.rs deleted file mode 100644 index 8a8958f3ad4..00000000000 --- a/src/tools/rustfmt/tests/source/postfix-yield.rs +++ /dev/null @@ -1,15 +0,0 @@ -// This demonstrates a proposed alternate or additional option of having yield in postfix position. -//@ edition: 2024 - -#![feature(gen_blocks, coroutines, coroutine_trait, yield_expr)] - -use std::ops::{Coroutine, CoroutineState}; -use std::pin::pin; - -fn main() { - let mut coro = - pin!(#[coroutine] |_: i32| { let x = 1.yield; - - - (x + 2).yield; }); -} diff --git a/src/tools/rustfmt/tests/target/postfix-yield.rs b/src/tools/rustfmt/tests/target/postfix-yield.rs index 7e94e1e095a..8ee34ec4312 100644 --- a/src/tools/rustfmt/tests/target/postfix-yield.rs +++ b/src/tools/rustfmt/tests/target/postfix-yield.rs @@ -7,6 +7,11 @@ use std::ops::{Coroutine, CoroutineState}; use std::pin::pin; fn main() { - let mut coro = - pin!(#[coroutine] |_: i32| { let x = 1.yield; (x + 2).yield; }); + let mut coro = pin!( + #[coroutine] + |_: i32| { + let x = 1.yield; + (x + 2).await; + } + ); } From 299e5d05147c3d3deefd3f85f6e994b5d05fb2f8 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Tue, 18 Mar 2025 10:50:33 -0700 Subject: [PATCH 06/16] Apply suggestions from code review Co-authored-by: Travis Cross --- compiler/rustc_ast_pretty/src/pprust/state/expr.rs | 2 +- compiler/rustc_parse/src/parser/expr.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs index 9e53ed41c46..caba5f2721a 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs @@ -774,7 +774,7 @@ impl<'a> State<'a> { } } ast::ExprKind::Yield(e, YieldKind::Postfix) => { - // it's not possible to have a postfix yield with no expression. + // It's not possible to have a postfix yield with no expression. let e = e.as_ref().unwrap(); self.print_expr_cond_paren( e, diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 28d100074f3..15a625314b4 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1311,7 +1311,7 @@ impl<'a> Parser<'a> { return self.parse_match_block(lo, match_span, self_arg, MatchKind::Postfix); } - // Post-fix yield + // Parse a postfix `yield`. if self.eat_keyword(exp!(Yield)) { let yield_span = self.prev_token.span; self.psess.gated_spans.gate(sym::yield_expr, yield_span); From 2bd7f73c2175c1f0ad56a0be4b5c39e2fc5ab97b Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Tue, 18 Mar 2025 12:19:43 -0700 Subject: [PATCH 07/16] Refactor YieldKind so postfix yield must have an expression --- compiler/rustc_ast/src/ast.rs | 37 +++++++++++++++++-- compiler/rustc_ast/src/mut_visit.rs | 7 +++- compiler/rustc_ast/src/util/classify.rs | 10 +++-- compiler/rustc_ast/src/visit.rs | 4 +- compiler/rustc_ast_lowering/src/expr.rs | 2 +- compiler/rustc_ast_lowering/src/format.rs | 2 +- .../rustc_ast_pretty/src/pprust/state/expr.rs | 6 +-- .../src/assert/context.rs | 2 +- compiler/rustc_parse/src/parser/expr.rs | 9 +++-- .../src/suspicious_operation_groupings.rs | 2 +- .../clippy/clippy_utils/src/ast_utils/mod.rs | 2 +- src/tools/rustfmt/src/chains.rs | 4 +- src/tools/rustfmt/src/expr.rs | 4 +- src/tools/rustfmt/src/utils.rs | 4 +- 14 files changed, 65 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 9dcdd868343..5b7545b3396 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1657,7 +1657,7 @@ pub enum ExprKind { Try(P), /// A `yield`, with an optional value to be yielded. - Yield(Option>, YieldKind), + Yield(YieldKind), /// A `do yeet` (aka `throw`/`fail`/`bail`/`raise`/whatever), /// with an optional value to be returned. @@ -1904,12 +1904,41 @@ pub enum MatchKind { } /// The kind of yield expression -#[derive(Clone, Copy, Encodable, Decodable, Debug, PartialEq)] +#[derive(Clone, Encodable, Decodable, Debug)] pub enum YieldKind { /// yield expr { ... } - Prefix, + Prefix(Option>), /// expr.yield { ... } - Postfix, + Postfix(P), +} + +impl YieldKind { + /// Returns the expression inside the yield expression, if any. + /// + /// For postfix yields, this is guaranteed to be `Some`. + pub const fn expr(&self) -> Option<&P> { + match self { + YieldKind::Prefix(expr) => expr.as_ref(), + YieldKind::Postfix(expr) => Some(expr), + } + } + + /// Returns a mutable reference to the expression being yielded, if any. + pub const fn expr_mut(&mut self) -> Option<&mut P> { + match self { + YieldKind::Prefix(expr) => expr.as_mut(), + YieldKind::Postfix(expr) => Some(expr), + } + } + + /// Returns true if both yields are prefix or both are postfix. + pub const fn same_kind(&self, other: &Self) -> bool { + match (self, other) { + (YieldKind::Prefix(_), YieldKind::Prefix(_)) => true, + (YieldKind::Postfix(_), YieldKind::Postfix(_)) => true, + _ => false, + } + } } /// A literal in a meta item. diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 5a9df6ffadf..b159e136245 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1813,8 +1813,11 @@ pub fn walk_expr(vis: &mut T, Expr { kind, id, span, attrs, token ExprKind::Paren(expr) => { vis.visit_expr(expr); } - ExprKind::Yield(expr, _) => { - visit_opt(expr, |expr| vis.visit_expr(expr)); + ExprKind::Yield(kind) => { + let expr = kind.expr_mut(); + if let Some(expr) = expr { + vis.visit_expr(expr); + } } ExprKind::Try(expr) => vis.visit_expr(expr), ExprKind::TryBlock(body) => vis.visit_block(body), diff --git a/compiler/rustc_ast/src/util/classify.rs b/compiler/rustc_ast/src/util/classify.rs index 116847af9f4..989ebe14bf8 100644 --- a/compiler/rustc_ast/src/util/classify.rs +++ b/compiler/rustc_ast/src/util/classify.rs @@ -182,11 +182,14 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option> { | Range(_, Some(e), _) | Ret(Some(e)) | Unary(_, e) - | Yield(Some(e), _) | Yeet(Some(e)) | Become(e) => { expr = e; } + Yield(kind) => match kind.expr() { + Some(e) => expr = e, + None => break None, + }, Closure(closure) => { expr = &closure.body; } @@ -217,7 +220,6 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option> { Break(_, None) | Range(_, None, _) | Ret(None) - | Yield(None, _) | Array(_) | Call(_, _) | MethodCall(_) @@ -237,7 +239,9 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option> { | Yeet(None) | UnsafeBinderCast(..) | Err(_) - | Dummy => break None, + | Dummy => { + break None; + } } } } diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index a72e834e17a..ce8d6df75af 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -1269,8 +1269,8 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) -> V try_visit!(visitor.visit_ty(container)); walk_list!(visitor, visit_ident, fields.iter()); } - ExprKind::Yield(optional_expression, _) => { - visit_opt!(visitor, visit_expr, optional_expression); + ExprKind::Yield(kind) => { + visit_opt!(visitor, visit_expr, kind.expr()); } ExprKind::Try(subexpression) => try_visit!(visitor.visit_expr(subexpression)), ExprKind::TryBlock(body) => try_visit!(visitor.visit_block(body)), diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 19a08f92ce7..7f3d060bb8a 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -351,7 +351,7 @@ impl<'hir> LoweringContext<'_, 'hir> { rest, ) } - ExprKind::Yield(opt_expr, _) => self.lower_expr_yield(e.span, opt_expr.as_deref()), + ExprKind::Yield(kind) => self.lower_expr_yield(e.span, kind.expr().map(|x| &**x)), ExprKind::Err(guar) => hir::ExprKind::Err(*guar), ExprKind::UnsafeBinderCast(kind, expr, ty) => hir::ExprKind::UnsafeBinderCast( diff --git a/compiler/rustc_ast_lowering/src/format.rs b/compiler/rustc_ast_lowering/src/format.rs index 2bbf957feeb..faa47274f96 100644 --- a/compiler/rustc_ast_lowering/src/format.rs +++ b/compiler/rustc_ast_lowering/src/format.rs @@ -642,7 +642,7 @@ fn may_contain_yield_point(e: &ast::Expr) -> bool { type Result = ControlFlow<()>; fn visit_expr(&mut self, e: &ast::Expr) -> ControlFlow<()> { - if let ast::ExprKind::Await(_, _) | ast::ExprKind::Yield(_, _) = e.kind { + if let ast::ExprKind::Await(_, _) | ast::ExprKind::Yield(_) = e.kind { ControlFlow::Break(()) } else { visit::walk_expr(self, e) diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs index caba5f2721a..7d9dc89bd75 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs @@ -761,7 +761,7 @@ impl<'a> State<'a> { self.print_expr(e, FixupContext::default()); self.pclose(); } - ast::ExprKind::Yield(e, YieldKind::Prefix) => { + ast::ExprKind::Yield(YieldKind::Prefix(e)) => { self.word("yield"); if let Some(expr) = e { @@ -773,9 +773,7 @@ impl<'a> State<'a> { ); } } - ast::ExprKind::Yield(e, YieldKind::Postfix) => { - // It's not possible to have a postfix yield with no expression. - let e = e.as_ref().unwrap(); + ast::ExprKind::Yield(YieldKind::Postfix(e)) => { self.print_expr_cond_paren( e, e.precedence() < ExprPrecedence::Unambiguous, diff --git a/compiler/rustc_builtin_macros/src/assert/context.rs b/compiler/rustc_builtin_macros/src/assert/context.rs index 54c3cebedfc..a949ab94f3a 100644 --- a/compiler/rustc_builtin_macros/src/assert/context.rs +++ b/compiler/rustc_builtin_macros/src/assert/context.rs @@ -323,7 +323,7 @@ impl<'cx, 'a> Context<'cx, 'a> { | ExprKind::While(_, _, _) | ExprKind::Yeet(_) | ExprKind::Become(_) - | ExprKind::Yield(_, _) + | ExprKind::Yield(_) | ExprKind::UnsafeBinderCast(..) => {} } } diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 15a625314b4..fc9a511d56a 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1315,8 +1315,9 @@ 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); - return Ok(self - .mk_expr(lo.to(yield_span), ExprKind::Yield(Some(self_arg), YieldKind::Postfix))); + return Ok( + self.mk_expr(lo.to(yield_span), ExprKind::Yield(YieldKind::Postfix(self_arg))) + ); } let fn_span_lo = self.token.span; @@ -1893,7 +1894,7 @@ impl<'a> Parser<'a> { /// Parse `"yield" expr?`. fn parse_expr_yield(&mut self) -> PResult<'a, P> { let lo = self.prev_token.span; - let kind = ExprKind::Yield(self.parse_expr_opt()?, YieldKind::Prefix); + let kind = ExprKind::Yield(YieldKind::Prefix(self.parse_expr_opt()?)); let span = lo.to(self.prev_token.span); self.psess.gated_spans.gate(sym::yield_expr, span); let expr = self.mk_expr(span, kind); @@ -4047,7 +4048,7 @@ impl MutVisitor for CondChecker<'_> { | ExprKind::MacCall(_) | ExprKind::Struct(_) | ExprKind::Repeat(_, _) - | ExprKind::Yield(_, _) + | ExprKind::Yield(_) | ExprKind::Yeet(_) | ExprKind::Become(_) | ExprKind::IncludedBytes(_) diff --git a/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs b/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs index 206912d8de4..0d809c17989 100644 --- a/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs +++ b/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs @@ -528,7 +528,7 @@ fn ident_difference_expr_with_base_location( &strip_non_ident_wrappers(left).kind, &strip_non_ident_wrappers(right).kind, ) { - (Yield(_, _), Yield(_, _)) + (Yield(_), Yield(_)) | (Try(_), Try(_)) | (Paren(_), Paren(_)) | (Repeat(_, _), Repeat(_, _)) diff --git a/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs b/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs index deda6030831..54261079fca 100644 --- a/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs +++ b/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs @@ -201,7 +201,7 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool { (Loop(lt, ll, _), Loop(rt, rl, _)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_block(lt, rt), (Block(lb, ll), Block(rb, rl)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_block(lb, rb), (TryBlock(l), TryBlock(r)) => eq_block(l, r), - (Yield(l, lk), Yield(r, rk)) => eq_expr_opt(l.as_ref(), r.as_ref()) && lk == rk, + (Yield(l), Yield(r)) => eq_expr_opt(l.expr(), r.expr()) && l.same_kind(r), (Ret(l), Ret(r)) => eq_expr_opt(l.as_ref(), r.as_ref()), (Break(ll, le), Break(rl, re)) => eq_label(ll.as_ref(), rl.as_ref()) && eq_expr_opt(le.as_ref(), re.as_ref()), (Continue(ll), Continue(rl)) => eq_label(ll.as_ref(), rl.as_ref()), diff --git a/src/tools/rustfmt/src/chains.rs b/src/tools/rustfmt/src/chains.rs index fabb4400553..034ecde068a 100644 --- a/src/tools/rustfmt/src/chains.rs +++ b/src/tools/rustfmt/src/chains.rs @@ -259,7 +259,7 @@ impl ChainItemKind { let span = mk_sp(nested.span.hi(), expr.span.hi()); (ChainItemKind::Await, span) } - ast::ExprKind::Yield(Some(ref nested), ast::YieldKind::Postfix) => { + ast::ExprKind::Yield(ast::YieldKind::Postfix(ref nested)) => { let span = mk_sp(nested.span.hi(), expr.span.hi()); (ChainItemKind::Yield, span) } @@ -516,7 +516,7 @@ impl Chain { ast::ExprKind::Field(ref subexpr, _) | ast::ExprKind::Try(ref subexpr) | ast::ExprKind::Await(ref subexpr, _) - | ast::ExprKind::Yield(Some(ref subexpr), ast::YieldKind::Postfix) => Some(SubExpr { + | ast::ExprKind::Yield(ast::YieldKind::Postfix(ref subexpr)) => Some(SubExpr { expr: Self::convert_try(subexpr, context), is_method_call_receiver: false, }), diff --git a/src/tools/rustfmt/src/expr.rs b/src/tools/rustfmt/src/expr.rs index 92c1ffa6076..e866f13efc7 100644 --- a/src/tools/rustfmt/src/expr.rs +++ b/src/tools/rustfmt/src/expr.rs @@ -221,7 +221,7 @@ pub(crate) fn format_expr( Ok(format!("break{id_str}")) } } - ast::ExprKind::Yield(ref opt_expr, ast::YieldKind::Prefix) => { + ast::ExprKind::Yield(ast::YieldKind::Prefix(ref opt_expr)) => { if let Some(ref expr) = *opt_expr { rewrite_unary_prefix(context, "yield ", &**expr, shape) } else { @@ -244,7 +244,7 @@ pub(crate) fn format_expr( | ast::ExprKind::Field(..) | ast::ExprKind::MethodCall(..) | ast::ExprKind::Await(_, _) - | ast::ExprKind::Yield(_, ast::YieldKind::Postfix) => rewrite_chain(expr, context, shape), + | ast::ExprKind::Yield(ast::YieldKind::Postfix(_)) => rewrite_chain(expr, context, shape), ast::ExprKind::MacCall(ref mac) => { rewrite_macro(mac, None, context, shape, MacroPosition::Expression).or_else(|_| { wrap_str( diff --git a/src/tools/rustfmt/src/utils.rs b/src/tools/rustfmt/src/utils.rs index 1811752c3c4..fcd475b1784 100644 --- a/src/tools/rustfmt/src/utils.rs +++ b/src/tools/rustfmt/src/utils.rs @@ -485,7 +485,7 @@ pub(crate) fn is_block_expr(context: &RewriteContext<'_>, expr: &ast::Expr, repr | ast::ExprKind::Index(_, ref expr, _) | ast::ExprKind::Unary(_, ref expr) | ast::ExprKind::Try(ref expr) - | ast::ExprKind::Yield(Some(ref expr), YieldKind::Prefix) => { + | ast::ExprKind::Yield(YieldKind::Prefix(Some(ref expr))) => { is_block_expr(context, expr, repr) } ast::ExprKind::Closure(ref closure) => is_block_expr(context, &closure.body, repr), @@ -517,7 +517,7 @@ pub(crate) fn is_block_expr(context: &RewriteContext<'_>, expr: &ast::Expr, repr | ast::ExprKind::Tup(..) | ast::ExprKind::Use(..) | ast::ExprKind::Type(..) - | ast::ExprKind::Yield(_, _) + | ast::ExprKind::Yield(..) | ast::ExprKind::Underscore => false, } } From f27cab806e5506fe27b71211af0ee3e9fa2ffdeb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 14 Mar 2025 09:03:23 +1100 Subject: [PATCH 08/16] Use `Option` for lowered param names. Parameter patterns are lowered to an `Ident` by `lower_fn_params_to_names`, which is used when lowering bare function types, trait methods, and foreign functions. Currently, there are two exceptional cases where the lowered param can become an empty `Ident`. - If the incoming pattern is an empty `Ident`. This occurs if the parameter is anonymous, e.g. in a bare function type. - If the incoming pattern is neither an ident nor an underscore. Any such parameter will have triggered a compile error (hence the `span_delayed_bug`), but lowering still occurs. This commit replaces these empty `Ident` results with `None`, which eliminates a number of `kw::Empty` uses, and makes it impossible to fail to check for these exceptional cases. Note: the `FIXME` comment in `is_unwrap_or_empty_symbol` is removed. It actually should have been removed in #138482, the precursor to this PR. That PR changed the lowering of wild patterns to `_` symbols instead of empty symbols, which made the mentioned underscore check load-bearing. --- compiler/rustc_ast_lowering/src/lib.rs | 14 ++++-- .../src/diagnostics/conflict_errors.rs | 12 ++--- .../rustc_borrowck/src/diagnostics/mod.rs | 2 +- compiler/rustc_hir/src/hir.rs | 13 ++++-- compiler/rustc_hir/src/intravisit.rs | 8 +++- .../src/check/compare_impl_item.rs | 8 +++- .../src/hir_ty_lowering/cmse.rs | 10 +++-- compiler/rustc_hir_pretty/src/lib.rs | 9 ++-- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 17 ++++--- .../rustc_hir_typeck/src/method/suggest.rs | 2 +- compiler/rustc_lint/src/nonstandard_style.rs | 4 +- compiler/rustc_metadata/src/rmeta/decoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/mod.rs | 2 +- compiler/rustc_middle/src/hir/map.rs | 8 ++-- compiler/rustc_middle/src/query/mod.rs | 2 +- .../rustc_resolve/src/late/diagnostics.rs | 11 +++-- .../src/error_reporting/traits/suggestions.rs | 9 ++-- src/librustdoc/clean/mod.rs | 24 +++++----- .../src/functions/renamed_function_params.rs | 45 ++++++++++--------- .../clippy/clippy_lints/src/lifetimes.rs | 10 ++--- 20 files changed, 125 insertions(+), 87 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index e24b45c5b19..e08850da4a7 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -1513,16 +1513,22 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { })) } - fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> &'hir [Ident] { + fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> &'hir [Option] { self.arena.alloc_from_iter(decl.inputs.iter().map(|param| match param.pat.kind { - PatKind::Ident(_, ident, _) => self.lower_ident(ident), - PatKind::Wild => Ident::new(kw::Underscore, self.lower_span(param.pat.span)), + PatKind::Ident(_, ident, _) => { + if ident.name != kw::Empty { + Some(self.lower_ident(ident)) + } else { + None + } + } + PatKind::Wild => Some(Ident::new(kw::Underscore, self.lower_span(param.pat.span))), _ => { self.dcx().span_delayed_bug( param.pat.span, "non-ident/wild param pat must trigger an error", ); - Ident::new(kw::Empty, self.lower_span(param.pat.span)) + None } })) } diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 2694a1eda78..978186f76a1 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -2514,12 +2514,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { let ty::Tuple(params) = tupled_params.kind() else { return }; // Find the first argument with a matching type, get its name - let Some((_, this_name)) = - params.iter().zip(tcx.hir_body_param_names(closure.body)).find(|(param_ty, name)| { + let Some(this_name) = params.iter().zip(tcx.hir_body_param_names(closure.body)).find_map( + |(param_ty, name)| { // FIXME: also support deref for stuff like `Rc` arguments - param_ty.peel_refs() == local_ty && name != &Ident::empty() - }) - else { + if param_ty.peel_refs() == local_ty { name } else { None } + }, + ) else { return; }; @@ -3787,7 +3787,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { method_args, *fn_span, call_source.from_hir_call(), - Some(self.infcx.tcx.fn_arg_names(method_did)[0]), + self.infcx.tcx.fn_arg_names(method_did)[0], ) { err.note(format!("borrow occurs due to deref coercion to `{deref_target_ty}`")); diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 208d510db2e..899e145c2c0 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -1029,7 +1029,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { method_args, *fn_span, call_source.from_hir_call(), - Some(self.infcx.tcx.fn_arg_names(method_did)[0]), + self.infcx.tcx.fn_arg_names(method_did)[0], ); return FnSelfUse { diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index b5857e359a2..751c379b21a 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2949,7 +2949,7 @@ impl<'hir> TraitItem<'hir> { #[derive(Debug, Clone, Copy, HashStable_Generic)] pub enum TraitFn<'hir> { /// No default body in the trait, just a signature. - Required(&'hir [Ident]), + Required(&'hir [Option]), /// Both signature and body are provided in the trait. Provided(BodyId), @@ -3354,7 +3354,9 @@ pub struct BareFnTy<'hir> { pub abi: ExternAbi, pub generic_params: &'hir [GenericParam<'hir>], pub decl: &'hir FnDecl<'hir>, - pub param_names: &'hir [Ident], + // `Option` because bare fn parameter names are optional. We also end up + // with `None` in some error cases, e.g. invalid parameter patterns. + pub param_names: &'hir [Option], } #[derive(Debug, Clone, Copy, HashStable_Generic)] @@ -4335,7 +4337,12 @@ impl ForeignItem<'_> { #[derive(Debug, Clone, Copy, HashStable_Generic)] pub enum ForeignItemKind<'hir> { /// A foreign function. - Fn(FnSig<'hir>, &'hir [Ident], &'hir Generics<'hir>), + /// + /// All argument idents are actually always present (i.e. `Some`), but + /// `&[Option]` is used because of code paths shared with `TraitFn` + /// and `BareFnTy`. The sharing is due to all of these cases not allowing + /// arbitrary patterns for parameters. + Fn(FnSig<'hir>, &'hir [Option], &'hir Generics<'hir>), /// A foreign static item (`static ext: u8`). Static(&'hir Ty<'hir>, Mutability, Safety), /// A foreign type. diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index b79ae1e7cc2..506358341b5 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -655,7 +655,9 @@ pub fn walk_foreign_item<'v, V: Visitor<'v>>( ForeignItemKind::Fn(ref sig, param_names, ref generics) => { try_visit!(visitor.visit_generics(generics)); try_visit!(visitor.visit_fn_decl(sig.decl)); - walk_list!(visitor, visit_ident, param_names.iter().copied()); + for ident in param_names.iter().copied() { + visit_opt!(visitor, visit_ident, ident); + } } ForeignItemKind::Static(ref typ, _, _) => { try_visit!(visitor.visit_ty_unambig(typ)); @@ -1169,7 +1171,9 @@ pub fn walk_trait_item<'v, V: Visitor<'v>>( } TraitItemKind::Fn(ref sig, TraitFn::Required(param_names)) => { try_visit!(visitor.visit_fn_decl(sig.decl)); - walk_list!(visitor, visit_ident, param_names.iter().copied()); + for ident in param_names.iter().copied() { + visit_opt!(visitor, visit_ident, ident); + } } TraitItemKind::Fn(ref sig, TraitFn::Provided(body_id)) => { try_visit!(visitor.visit_fn( diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index ca820deebdf..84d07c711fa 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -1034,7 +1034,13 @@ fn report_trait_method_mismatch<'tcx>( let span = tcx .hir_body_param_names(body) .zip(sig.decl.inputs.iter()) - .map(|(param, ty)| param.span.to(ty.span)) + .map(|(param_name, ty)| { + if let Some(param_name) = param_name { + param_name.span.to(ty.span) + } else { + ty.span + } + }) .next() .unwrap_or(impl_err_span); diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs index 5fed2e35287..170500c7a16 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/cmse.rs @@ -49,10 +49,12 @@ pub(crate) fn validate_cmse_abi<'tcx>( Ok(Err(index)) => { // fn(x: u32, u32, u32, u16, y: u16) -> u32, // ^^^^^^ - let span = bare_fn_ty.param_names[index] - .span - .to(bare_fn_ty.decl.inputs[index].span) - .to(bare_fn_ty.decl.inputs.last().unwrap().span); + let span = if let Some(ident) = bare_fn_ty.param_names[index] { + ident.span.to(bare_fn_ty.decl.inputs[index].span) + } else { + bare_fn_ty.decl.inputs[index].span + } + .to(bare_fn_ty.decl.inputs.last().unwrap().span); let plural = bare_fn_ty.param_names.len() - index != 1; dcx.emit_err(errors::CmseInputsStackSpill { span, plural, abi }); } diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs index 98b81dd3def..ddaca89ccf8 100644 --- a/compiler/rustc_hir_pretty/src/lib.rs +++ b/compiler/rustc_hir_pretty/src/lib.rs @@ -2,6 +2,7 @@ //! the definitions in this file have equivalents in `rustc_ast_pretty`. // tidy-alphabetical-start +#![feature(let_chains)] #![recursion_limit = "256"] // tidy-alphabetical-end @@ -898,7 +899,7 @@ impl<'a> State<'a> { ident: Ident, m: &hir::FnSig<'_>, generics: &hir::Generics<'_>, - arg_names: &[Ident], + arg_names: &[Option], body_id: Option, ) { self.print_fn(m.decl, m.header, Some(ident.name), generics, arg_names, body_id); @@ -2121,7 +2122,7 @@ impl<'a> State<'a> { header: hir::FnHeader, name: Option, generics: &hir::Generics<'_>, - arg_names: &[Ident], + arg_names: &[Option], body_id: Option, ) { self.print_fn_header_info(header); @@ -2141,7 +2142,7 @@ impl<'a> State<'a> { s.print_implicit_self(&decl.implicit_self); } else { if let Some(arg_name) = arg_names.get(i) { - if arg_name.name != kw::Empty { + if let Some(arg_name) = arg_name { s.word(arg_name.to_string()); s.word(":"); s.space(); @@ -2451,7 +2452,7 @@ impl<'a> State<'a> { decl: &hir::FnDecl<'_>, name: Option, generic_params: &[hir::GenericParam<'_>], - arg_names: &[Ident], + arg_names: &[Option], ) { self.ibox(INDENT_UNIT); self.print_formal_generic_params(generic_params); diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 90bdb3c4b37..f4bd7ec701f 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -1135,7 +1135,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { && self.tcx.def_kind(fn_def_id).is_fn_like() && let self_implicit = matches!(call_expr.kind, hir::ExprKind::MethodCall(..)) as usize - && let Some(arg) = + && let Some(Some(arg)) = self.tcx.fn_arg_names(fn_def_id).get(expected_idx.as_usize() + self_implicit) && arg.name != kw::SelfLower { @@ -2678,7 +2678,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { params.get(is_method as usize..params.len() - sig.decl.c_variadic as usize)?; debug_assert_eq!(params.len(), fn_inputs.len()); Some(( - fn_inputs.zip(params.iter().map(|¶m| FnParam::Name(param))).collect(), + fn_inputs.zip(params.iter().map(|&ident| FnParam::Name(ident))).collect(), generics, )) } @@ -2709,14 +2709,20 @@ impl<'tcx> Visitor<'tcx> for FindClosureArg<'tcx> { #[derive(Clone, Copy)] enum FnParam<'hir> { Param(&'hir hir::Param<'hir>), - Name(Ident), + Name(Option), } impl FnParam<'_> { fn span(&self) -> Span { match self { Self::Param(param) => param.span, - Self::Name(ident) => ident.span, + Self::Name(ident) => { + if let Some(ident) = ident { + ident.span + } else { + DUMMY_SP + } + } } } @@ -2733,7 +2739,8 @@ impl FnParam<'_> { Some(ident.name) } FnParam::Name(ident) - if ident.name != kw::Empty && ident.name != kw::Underscore => + if let Some(ident) = ident + && ident.name != kw::Underscore => { Some(ident.name) } diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index cdfae51583b..908c3ee2eb8 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -3766,7 +3766,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { { let self_first_arg = match method { hir::TraitFn::Required([ident, ..]) => { - ident.name == kw::SelfLower + matches!(ident, Some(Ident { name: kw::SelfLower, .. })) } hir::TraitFn::Provided(body_id) => { self.tcx.hir_body(*body_id).params.first().is_some_and( diff --git a/compiler/rustc_lint/src/nonstandard_style.rs b/compiler/rustc_lint/src/nonstandard_style.rs index 715e3506ab8..752636ccaf0 100644 --- a/compiler/rustc_lint/src/nonstandard_style.rs +++ b/compiler/rustc_lint/src/nonstandard_style.rs @@ -424,7 +424,9 @@ impl<'tcx> LateLintPass<'tcx> for NonSnakeCase { if let hir::TraitItemKind::Fn(_, hir::TraitFn::Required(pnames)) = item.kind { self.check_snake_case(cx, "trait method", &item.ident); for param_name in pnames { - self.check_snake_case(cx, "variable", param_name); + if let Some(param_name) = param_name { + self.check_snake_case(cx, "variable", param_name); + } } } } diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index e60904eebeb..24248e8dde3 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1318,7 +1318,7 @@ impl<'a> CrateMetadataRef<'a> { .expect("argument names not encoded for a function") .decode((self, sess)) .nth(0) - .is_some_and(|ident| ident.name == kw::SelfLower) + .is_some_and(|ident| matches!(ident, Some(Ident { name: kw::SelfLower, .. }))) } fn get_associated_item_or_field_def_ids(self, id: DefIndex) -> impl Iterator { diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 5536c93f84a..dc453b1e747 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -443,7 +443,7 @@ define_tables! { rendered_const: Table>, rendered_precise_capturing_args: Table>>, asyncness: Table, - fn_arg_names: Table>, + fn_arg_names: Table>>, coroutine_kind: Table, coroutine_for_closure: Table, coroutine_by_move_body_def_id: Table, diff --git a/compiler/rustc_middle/src/hir/map.rs b/compiler/rustc_middle/src/hir/map.rs index 2e589150d3e..f17efab81ec 100644 --- a/compiler/rustc_middle/src/hir/map.rs +++ b/compiler/rustc_middle/src/hir/map.rs @@ -280,11 +280,11 @@ impl<'tcx> TyCtxt<'tcx> { }) } - pub fn hir_body_param_names(self, id: BodyId) -> impl Iterator { + pub fn hir_body_param_names(self, id: BodyId) -> impl Iterator> { self.hir_body(id).params.iter().map(|param| match param.pat.kind { - PatKind::Binding(_, _, ident, _) => ident, - PatKind::Wild => Ident::new(kw::Underscore, param.pat.span), - _ => Ident::empty(), + PatKind::Binding(_, _, ident, _) => Some(ident), + PatKind::Wild => Some(Ident::new(kw::Underscore, param.pat.span)), + _ => None, }) } diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 94a5a3769a3..527c18addbe 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1410,7 +1410,7 @@ rustc_queries! { desc { |tcx| "computing target features for inline asm of `{}`", tcx.def_path_str(def_id) } } - query fn_arg_names(def_id: DefId) -> &'tcx [rustc_span::Ident] { + query fn_arg_names(def_id: DefId) -> &'tcx [Option] { desc { |tcx| "looking up function parameter names for `{}`", tcx.def_path_str(def_id) } separate_provide_extern } diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index bcfcc8000c7..3d666055a94 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2217,12 +2217,11 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { .delegation_fn_sigs .get(&def_id) .is_some_and(|sig| sig.has_self), - None => self - .r - .tcx - .fn_arg_names(def_id) - .first() - .is_some_and(|ident| ident.name == kw::SelfLower), + None => { + self.r.tcx.fn_arg_names(def_id).first().is_some_and(|&ident| { + matches!(ident, Some(Ident { name: kw::SelfLower, .. })) + }) + } }; if has_self { return Some(AssocSuggestion::MethodWithSelf { called }); diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs index 393d175ea4c..dc8022b95c3 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs @@ -1992,13 +1992,12 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { .iter() .enumerate() .map(|(i, ident)| { - if ident.name.is_empty() - || ident.name == kw::Underscore - || ident.name == kw::SelfLower + if let Some(ident) = ident + && !matches!(ident, Ident { name: kw::Underscore | kw::SelfLower, .. }) { - format!("arg{i}") - } else { format!("{ident}") + } else { + format!("arg{i}") } }) .collect(); diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 4ecf702d7b6..de6dc088176 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1088,7 +1088,7 @@ fn clean_fn_decl_legacy_const_generics(func: &mut Function, attrs: &[hir::Attrib enum FunctionArgs<'tcx> { Body(hir::BodyId), - Names(&'tcx [Ident]), + Names(&'tcx [Option]), } fn clean_function<'tcx>( @@ -1117,13 +1117,15 @@ fn clean_function<'tcx>( fn clean_args_from_types_and_names<'tcx>( cx: &mut DocContext<'tcx>, types: &[hir::Ty<'tcx>], - names: &[Ident], + names: &[Option], ) -> Arguments { - fn nonempty_name(ident: &Ident) -> Option { - if ident.name == kw::Underscore || ident.name == kw::Empty { - None - } else { + fn nonempty_name(ident: &Option) -> Option { + if let Some(ident) = ident + && ident.name != kw::Underscore + { Some(ident.name) + } else { + None } } @@ -1216,11 +1218,11 @@ fn clean_poly_fn_sig<'tcx>( .iter() .map(|t| Argument { type_: clean_middle_ty(t.map_bound(|t| *t), cx, None, None), - name: names - .next() - .map(|i| i.name) - .filter(|i| !i.is_empty()) - .unwrap_or(kw::Underscore), + name: if let Some(Some(ident)) = names.next() { + ident.name + } else { + kw::Underscore + }, is_const: false, }) .collect(), diff --git a/src/tools/clippy/clippy_lints/src/functions/renamed_function_params.rs b/src/tools/clippy/clippy_lints/src/functions/renamed_function_params.rs index 5ad83f886e2..041f6228fba 100644 --- a/src/tools/clippy/clippy_lints/src/functions/renamed_function_params.rs +++ b/src/tools/clippy/clippy_lints/src/functions/renamed_function_params.rs @@ -5,7 +5,7 @@ use rustc_hir::hir_id::OwnerId; use rustc_hir::{Impl, ImplItem, ImplItemKind, ImplItemRef, ItemKind, Node, TraitRef}; use rustc_lint::LateContext; use rustc_span::Span; -use rustc_span::symbol::{Ident, Symbol, kw}; +use rustc_span::symbol::{Ident, kw}; use super::RENAMED_FUNCTION_PARAMS; @@ -51,22 +51,33 @@ struct RenamedFnArgs(Vec<(Span, String)>); impl RenamedFnArgs { /// Comparing between an iterator of default names and one with current names, /// then collect the ones that got renamed. - fn new(default_names: &mut I, current_names: &mut T) -> Self + fn new(default_idents: &mut I1, current_idents: &mut I2) -> Self where - I: Iterator, - T: Iterator, + I1: Iterator>, + I2: Iterator>, { let mut renamed: Vec<(Span, String)> = vec![]; - debug_assert!(default_names.size_hint() == current_names.size_hint()); - while let (Some(def_name), Some(cur_name)) = (default_names.next(), current_names.next()) { - let current_name = cur_name.name; - let default_name = def_name.name; - if is_unused_or_empty_symbol(current_name) || is_unused_or_empty_symbol(default_name) { - continue; - } - if current_name != default_name { - renamed.push((cur_name.span, default_name.to_string())); + debug_assert!(default_idents.size_hint() == current_idents.size_hint()); + while let (Some(default_ident), Some(current_ident)) = + (default_idents.next(), current_idents.next()) + { + let has_name_to_check = |ident: Option| { + if let Some(ident) = ident + && ident.name != kw::Underscore + && !ident.name.as_str().starts_with('_') + { + Some(ident) + } else { + None + } + }; + + if let Some(default_ident) = has_name_to_check(default_ident) + && let Some(current_ident) = has_name_to_check(current_ident) + && default_ident.name != current_ident.name + { + renamed.push((current_ident.span, default_ident.to_string())); } } @@ -83,14 +94,6 @@ impl RenamedFnArgs { } } -fn is_unused_or_empty_symbol(symbol: Symbol) -> bool { - // FIXME: `body_param_names` currently returning empty symbols for `wild` as well, - // so we need to check if the symbol is empty first. - // Therefore the check of whether it's equal to [`kw::Underscore`] has no use for now, - // but it would be nice to keep it here just to be future-proof. - symbol.is_empty() || symbol == kw::Underscore || symbol.as_str().starts_with('_') -} - /// Get the [`trait_item_def_id`](ImplItemRef::trait_item_def_id) of a relevant impl item. fn trait_item_def_id_of_impl(items: &[ImplItemRef], target: OwnerId) -> Option { items.iter().find_map(|item| { diff --git a/src/tools/clippy/clippy_lints/src/lifetimes.rs b/src/tools/clippy/clippy_lints/src/lifetimes.rs index 3dd2de1fafc..8d47c756fc5 100644 --- a/src/tools/clippy/clippy_lints/src/lifetimes.rs +++ b/src/tools/clippy/clippy_lints/src/lifetimes.rs @@ -189,7 +189,7 @@ fn check_fn_inner<'tcx>( cx: &LateContext<'tcx>, sig: &'tcx FnSig<'_>, body: Option, - trait_sig: Option<&[Ident]>, + trait_sig: Option<&[Option]>, generics: &'tcx Generics<'_>, span: Span, report_extra_lifetimes: bool, @@ -264,7 +264,7 @@ fn could_use_elision<'tcx>( cx: &LateContext<'tcx>, func: &'tcx FnDecl<'_>, body: Option, - trait_sig: Option<&[Ident]>, + trait_sig: Option<&[Option]>, named_generics: &'tcx [GenericParam<'_>], msrv: Msrv, ) -> Option<(Vec, Vec)> { @@ -310,7 +310,7 @@ fn could_use_elision<'tcx>( let body = cx.tcx.hir_body(body_id); let first_ident = body.params.first().and_then(|param| param.pat.simple_ident()); - if non_elidable_self_type(cx, func, first_ident, msrv) { + if non_elidable_self_type(cx, func, Some(first_ident), msrv) { return None; } @@ -384,8 +384,8 @@ fn allowed_lts_from(named_generics: &[GenericParam<'_>]) -> FxIndexSet(cx: &LateContext<'tcx>, func: &FnDecl<'tcx>, ident: Option, msrv: Msrv) -> bool { - if let Some(ident) = ident +fn non_elidable_self_type<'tcx>(cx: &LateContext<'tcx>, func: &FnDecl<'tcx>, ident: Option>, msrv: Msrv) -> bool { + if let Some(Some(ident)) = ident && ident.name == kw::SelfLower && !func.implicit_self.has_implicit_self() && let Some(self_ty) = func.inputs.first() From 5a52b5d92aec9427b40aee6a20093eb822702402 Mon Sep 17 00:00:00 2001 From: xizheyin Date: Wed, 19 Mar 2025 18:45:16 +0800 Subject: [PATCH 09/16] Suggest `-Whelp` when pass `--print lints` to rustc Signed-off-by: xizheyin --- compiler/rustc_session/src/config.rs | 6 ++++++ tests/ui/invalid-compile-flags/print.stderr | 1 + tests/ui/rustc-print-info-issue-138612.rs | 2 ++ tests/ui/rustc-print-info-issue-138612.stderr | 6 ++++++ 4 files changed, 15 insertions(+) create mode 100644 tests/ui/rustc-print-info-issue-138612.rs create mode 100644 tests/ui/rustc-print-info-issue-138612.stderr diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 701d06e4fd4..43b78423c72 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2082,6 +2082,12 @@ fn emit_unknown_print_request_help(early_dcx: &EarlyDiagCtxt, req: &str) -> ! { let mut diag = early_dcx.early_struct_fatal(format!("unknown print request: `{req}`")); #[allow(rustc::diagnostic_outside_of_impl)] diag.help(format!("valid print requests are: {prints}")); + + if req == "lints" { + diag.help(format!("use `-Whelp` to print a list of lints")); + } + + diag.help(format!("for more information, see the rustc book: https://doc.rust-lang.org/rustc/command-line-arguments.html#--print-print-compiler-information")); diag.emit() } diff --git a/tests/ui/invalid-compile-flags/print.stderr b/tests/ui/invalid-compile-flags/print.stderr index df0c3977dc8..4ea06a06539 100644 --- a/tests/ui/invalid-compile-flags/print.stderr +++ b/tests/ui/invalid-compile-flags/print.stderr @@ -1,4 +1,5 @@ error: unknown print request: `yyyy` | = help: valid print requests are: `all-target-specs-json`, `calling-conventions`, `cfg`, `check-cfg`, `code-models`, `crate-name`, `deployment-target`, `file-names`, `host-tuple`, `link-args`, `native-static-libs`, `relocation-models`, `split-debuginfo`, `stack-protector-strategies`, `sysroot`, `target-cpus`, `target-features`, `target-libdir`, `target-list`, `target-spec-json`, `tls-models` + = help: for more information, see the rustc book: https://doc.rust-lang.org/rustc/command-line-arguments.html#--print-print-compiler-information diff --git a/tests/ui/rustc-print-info-issue-138612.rs b/tests/ui/rustc-print-info-issue-138612.rs new file mode 100644 index 00000000000..65b595635b1 --- /dev/null +++ b/tests/ui/rustc-print-info-issue-138612.rs @@ -0,0 +1,2 @@ +//@ check-fail +//@ compile-flags: /dev/null --print lints diff --git a/tests/ui/rustc-print-info-issue-138612.stderr b/tests/ui/rustc-print-info-issue-138612.stderr new file mode 100644 index 00000000000..4f7ed821952 --- /dev/null +++ b/tests/ui/rustc-print-info-issue-138612.stderr @@ -0,0 +1,6 @@ +error: unknown print request: `lints` + | + = help: valid print requests are: `all-target-specs-json`, `calling-conventions`, `cfg`, `check-cfg`, `code-models`, `crate-name`, `deployment-target`, `file-names`, `host-tuple`, `link-args`, `native-static-libs`, `relocation-models`, `split-debuginfo`, `stack-protector-strategies`, `sysroot`, `target-cpus`, `target-features`, `target-libdir`, `target-list`, `target-spec-json`, `tls-models` + = help: use `-Whelp` to print a list of lints + = help: for more information, see the rustc book: https://doc.rust-lang.org/rustc/command-line-arguments.html#--print-print-compiler-information + From d07ef5b0e1f7551ad24f86256d63a5dfdb907c17 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 4 Mar 2025 21:55:43 +1100 Subject: [PATCH 10/16] coverage: Add LLVM plumbing for expansion regions This is currently unused, but paves the way for future work on expansion regions without having to worry about the FFI parts. --- .../src/coverageinfo/ffi.rs | 19 +++++++++++++++++-- .../src/coverageinfo/llvm_cov.rs | 16 ++++++++++++++-- .../src/coverageinfo/mapgen/covfun.rs | 11 ++++++++--- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 2 ++ .../llvm-wrapper/CoverageMappingWrapper.cpp | 16 ++++++++++++++++ 5 files changed, 57 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index b617f4d37f5..f6000e72840 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -146,6 +146,7 @@ pub(crate) struct CoverageSpan { #[derive(Clone, Debug, Default)] pub(crate) struct Regions { pub(crate) code_regions: Vec, + pub(crate) expansion_regions: Vec, pub(crate) branch_regions: Vec, pub(crate) mcdc_branch_regions: Vec, pub(crate) mcdc_decision_regions: Vec, @@ -154,10 +155,16 @@ pub(crate) struct Regions { impl Regions { /// Returns true if none of this structure's tables contain any regions. pub(crate) fn has_no_regions(&self) -> bool { - let Self { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } = - self; + let Self { + code_regions, + expansion_regions, + branch_regions, + mcdc_branch_regions, + mcdc_decision_regions, + } = self; code_regions.is_empty() + && expansion_regions.is_empty() && branch_regions.is_empty() && mcdc_branch_regions.is_empty() && mcdc_decision_regions.is_empty() @@ -172,6 +179,14 @@ pub(crate) struct CodeRegion { pub(crate) counter: Counter, } +/// Must match the layout of `LLVMRustCoverageExpansionRegion`. +#[derive(Clone, Debug)] +#[repr(C)] +pub(crate) struct ExpansionRegion { + pub(crate) cov_span: CoverageSpan, + pub(crate) expanded_file_id: u32, +} + /// Must match the layout of `LLVMRustCoverageBranchRegion`. #[derive(Clone, Debug)] #[repr(C)] diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs index 2cd7fa3225a..907d6d41a1f 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs @@ -63,8 +63,18 @@ pub(crate) fn write_function_mappings_to_buffer( expressions: &[ffi::CounterExpression], regions: &ffi::Regions, ) -> Vec { - let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } = - regions; + let ffi::Regions { + code_regions, + expansion_regions, + branch_regions, + mcdc_branch_regions, + mcdc_decision_regions, + } = regions; + + // SAFETY: + // - All types are FFI-compatible and have matching representations in Rust/C++. + // - For pointer/length pairs, the pointer and length come from the same vector or slice. + // - C++ code does not retain any pointers after the call returns. llvm::build_byte_buffer(|buffer| unsafe { llvm::LLVMRustCoverageWriteFunctionMappingsToBuffer( virtual_file_mapping.as_ptr(), @@ -73,6 +83,8 @@ pub(crate) fn write_function_mappings_to_buffer( expressions.len(), code_regions.as_ptr(), code_regions.len(), + expansion_regions.as_ptr(), + expansion_regions.len(), branch_regions.as_ptr(), branch_regions.len(), mcdc_branch_regions.as_ptr(), diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index 5b487bc1a8b..b8082edb9d5 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -120,13 +120,18 @@ fn fill_region_tables<'tcx>( // Associate that global file ID with a local file ID for this function. let local_file_id = covfun.virtual_file_mapping.local_id_for_global(global_file_id); - let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } = - &mut covfun.regions; - let make_cov_span = |span: Span| spans::make_coverage_span(local_file_id, source_map, &source_file, span); let discard_all = tcx.sess.coverage_discard_all_spans_in_codegen(); + let ffi::Regions { + code_regions, + expansion_regions: _, // FIXME(Zalathar): Fill out support for expansion regions + branch_regions, + mcdc_branch_regions, + mcdc_decision_regions, + } = &mut covfun.regions; + // For each counter/region pair in this function+file, convert it to a // form suitable for FFI. for &Mapping { ref kind, span } in &fn_cov_info.mappings { diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 39087a4d6f4..83efb3ea660 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -2019,6 +2019,8 @@ unsafe extern "C" { NumExpressions: size_t, CodeRegions: *const crate::coverageinfo::ffi::CodeRegion, NumCodeRegions: size_t, + ExpansionRegions: *const crate::coverageinfo::ffi::ExpansionRegion, + NumExpansionRegions: size_t, BranchRegions: *const crate::coverageinfo::ffi::BranchRegion, NumBranchRegions: size_t, MCDCBranchRegions: *const crate::coverageinfo::ffi::MCDCBranchRegion, diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 0471baa1f9c..b8884486c33 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -77,6 +77,13 @@ struct LLVMRustCoverageCodeRegion { LLVMRustCounter Count; }; +// Must match the layout of +// `rustc_codegen_llvm::coverageinfo::ffi::ExpansionRegion`. +struct LLVMRustCoverageExpansionRegion { + LLVMRustCoverageSpan Span; + uint32_t ExpandedFileID; +}; + // Must match the layout of // `rustc_codegen_llvm::coverageinfo::ffi::BranchRegion`. struct LLVMRustCoverageBranchRegion { @@ -151,6 +158,8 @@ extern "C" void LLVMRustCoverageWriteFunctionMappingsToBuffer( const unsigned *VirtualFileMappingIDs, size_t NumVirtualFileMappingIDs, const LLVMRustCounterExpression *RustExpressions, size_t NumExpressions, const LLVMRustCoverageCodeRegion *CodeRegions, size_t NumCodeRegions, + const LLVMRustCoverageExpansionRegion *ExpansionRegions, + size_t NumExpansionRegions, const LLVMRustCoverageBranchRegion *BranchRegions, size_t NumBranchRegions, const LLVMRustCoverageMCDCBranchRegion *MCDCBranchRegions, size_t NumMCDCBranchRegions, @@ -179,6 +188,13 @@ extern "C" void LLVMRustCoverageWriteFunctionMappingsToBuffer( Region.Span.ColumnStart, Region.Span.LineEnd, Region.Span.ColumnEnd)); } + // Expansion regions: + for (const auto &Region : ArrayRef(ExpansionRegions, NumExpansionRegions)) { + MappingRegions.push_back(coverage::CounterMappingRegion::makeExpansion( + Region.Span.FileID, Region.ExpandedFileID, Region.Span.LineStart, + Region.Span.ColumnStart, Region.Span.LineEnd, Region.Span.ColumnEnd)); + } + // Branch regions: for (const auto &Region : ArrayRef(BranchRegions, NumBranchRegions)) { MappingRegions.push_back(coverage::CounterMappingRegion::makeBranchRegion( From 220851cc75558d06df98c7650b50d59ef2b5e348 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 20 Mar 2025 01:59:38 +0000 Subject: [PATCH 11/16] Do not rely on type_var_origin in OrphanCheckErr::NonLocalInputType --- .../src/coherence/orphan.rs | 63 +++++-------------- .../rustc_next_trait_solver/src/coherence.rs | 5 +- compiler/rustc_type_ir/src/macros.rs | 13 ++-- tests/crashes/132826.rs | 10 --- .../orphan-check-error-reporting-ty-var.rs | 17 +++++ ...orphan-check-error-reporting-ty-var.stderr | 16 +++++ 6 files changed, 60 insertions(+), 64 deletions(-) delete mode 100644 tests/crashes/132826.rs create mode 100644 tests/ui/coherence/orphan-check-error-reporting-ty-var.rs create mode 100644 tests/ui/coherence/orphan-check-error-reporting-ty-var.stderr diff --git a/compiler/rustc_hir_analysis/src/coherence/orphan.rs b/compiler/rustc_hir_analysis/src/coherence/orphan.rs index 0b7fc44460e..74ba4ffe25e 100644 --- a/compiler/rustc_hir_analysis/src/coherence/orphan.rs +++ b/compiler/rustc_hir_analysis/src/coherence/orphan.rs @@ -3,11 +3,10 @@ use rustc_data_structures::fx::FxIndexSet; use rustc_errors::ErrorGuaranteed; -use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; +use rustc_infer::infer::{DefineOpaqueTypes, InferCtxt, TyCtxtInferExt}; use rustc_lint_defs::builtin::UNCOVERED_PARAM_IN_PROJECTION; use rustc_middle::ty::{ - self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeSuperVisitable, - TypeVisitable, TypeVisitableExt, TypeVisitor, TypingMode, + self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, TypingMode, }; use rustc_middle::{bug, span_bug}; use rustc_span::def_id::{DefId, LocalDefId}; @@ -356,13 +355,20 @@ fn orphan_check<'tcx>( }) } OrphanCheckErr::NonLocalInputType(tys) => { - let generics = tcx.generics_of(impl_def_id); - let tys = tys - .into_iter() - .map(|(ty, is_target_ty)| { - (ty.fold_with(&mut TyVarReplacer { infcx: &infcx, generics }), is_target_ty) - }) - .collect(); + let tys = infcx.probe(|_| { + // Map the unconstrained args back to their params, + // ignoring any type unification errors. + for (arg, id_arg) in + std::iter::zip(args, ty::GenericArgs::identity_for_item(tcx, impl_def_id)) + { + let _ = infcx.at(&cause, ty::ParamEnv::empty()).eq( + DefineOpaqueTypes::No, + arg, + id_arg, + ); + } + infcx.resolve_vars_if_possible(tys) + }); OrphanCheckErr::NonLocalInputType(tys) } }) @@ -536,40 +542,3 @@ impl<'tcx> TypeVisitor> for UncoveredTyParamCollector<'_, 'tcx> { } } } - -struct TyVarReplacer<'cx, 'tcx> { - infcx: &'cx InferCtxt<'tcx>, - generics: &'tcx ty::Generics, -} - -impl<'cx, 'tcx> TypeFolder> for TyVarReplacer<'cx, 'tcx> { - fn cx(&self) -> TyCtxt<'tcx> { - self.infcx.tcx - } - - fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> { - if !ty.has_type_flags(ty::TypeFlags::HAS_TY_INFER) { - return ty; - } - let ty::Infer(ty::TyVar(vid)) = *ty.kind() else { - return ty.super_fold_with(self); - }; - let origin = self.infcx.type_var_origin(vid); - if let Some(def_id) = origin.param_def_id { - // The generics of an `impl` don't have a parent, we can index directly. - let index = self.generics.param_def_id_to_index[&def_id]; - let name = self.generics.own_params[index as usize].name; - - Ty::new_param(self.infcx.tcx, index, name) - } else { - ty - } - } - - fn fold_const(&mut self, ct: ty::Const<'tcx>) -> ty::Const<'tcx> { - if !ct.has_type_flags(ty::TypeFlags::HAS_TY_INFER) { - return ct; - } - ct.super_fold_with(self) - } -} diff --git a/compiler/rustc_next_trait_solver/src/coherence.rs b/compiler/rustc_next_trait_solver/src/coherence.rs index 53290203600..f8215a228f5 100644 --- a/compiler/rustc_next_trait_solver/src/coherence.rs +++ b/compiler/rustc_next_trait_solver/src/coherence.rs @@ -4,7 +4,8 @@ use std::ops::ControlFlow; use derive_where::derive_where; use rustc_type_ir::inherent::*; use rustc_type_ir::{ - self as ty, InferCtxtLike, Interner, TypeVisitable, TypeVisitableExt, TypeVisitor, + self as ty, InferCtxtLike, Interner, TrivialTypeTraversalImpls, TypeVisitable, + TypeVisitableExt, TypeVisitor, }; use tracing::instrument; @@ -95,6 +96,8 @@ pub fn trait_ref_is_local_or_fundamental(tcx: I, trait_ref: ty::Tra trait_ref.def_id.is_local() || tcx.trait_is_fundamental(trait_ref.def_id) } +TrivialTypeTraversalImpls! { IsFirstInputType, } + #[derive(Debug, Copy, Clone)] pub enum IsFirstInputType { No, diff --git a/compiler/rustc_type_ir/src/macros.rs b/compiler/rustc_type_ir/src/macros.rs index fab1a11304d..c8c293121ca 100644 --- a/compiler/rustc_type_ir/src/macros.rs +++ b/compiler/rustc_type_ir/src/macros.rs @@ -1,10 +1,11 @@ /// Used for types that are `Copy` and which **do not care arena /// allocated data** (i.e., don't need to be folded). +#[macro_export] macro_rules! TrivialTypeTraversalImpls { ($($ty:ty,)+) => { $( - impl $crate::fold::TypeFoldable for $ty { - fn try_fold_with>( + impl $crate::TypeFoldable for $ty { + fn try_fold_with>( self, _: &mut F, ) -> ::std::result::Result { @@ -12,7 +13,7 @@ macro_rules! TrivialTypeTraversalImpls { } #[inline] - fn fold_with>( + fn fold_with>( self, _: &mut F, ) -> Self { @@ -20,14 +21,14 @@ macro_rules! TrivialTypeTraversalImpls { } } - impl $crate::visit::TypeVisitable for $ty { + impl $crate::TypeVisitable for $ty { #[inline] - fn visit_with>( + fn visit_with>( &self, _: &mut F) -> F::Result { - ::output() + ::output() } } )+ diff --git a/tests/crashes/132826.rs b/tests/crashes/132826.rs deleted file mode 100644 index 9889cecdac5..00000000000 --- a/tests/crashes/132826.rs +++ /dev/null @@ -1,10 +0,0 @@ -//@ known-bug: #132826 -pub trait MyTrait { - type Item; -} - -impl MyTrait for Vec { - type Item = Vec; -} - -impl From> for as MyTrait>::Item {} diff --git a/tests/ui/coherence/orphan-check-error-reporting-ty-var.rs b/tests/ui/coherence/orphan-check-error-reporting-ty-var.rs new file mode 100644 index 00000000000..99a83453354 --- /dev/null +++ b/tests/ui/coherence/orphan-check-error-reporting-ty-var.rs @@ -0,0 +1,17 @@ +// Regression test for #132826. + +// Make sure we don't try to resolve the variable `K1` in the generics of the impl +// (which only has `K2`). + +pub trait MyTrait { + type Item; +} + +impl MyTrait for Vec { + type Item = Vec; +} + +impl From> for as MyTrait>::Item {} +//~^ ERROR only traits defined in the current crate can be implemented for arbitrary types + +fn main() {} diff --git a/tests/ui/coherence/orphan-check-error-reporting-ty-var.stderr b/tests/ui/coherence/orphan-check-error-reporting-ty-var.stderr new file mode 100644 index 00000000000..f229f8b2e38 --- /dev/null +++ b/tests/ui/coherence/orphan-check-error-reporting-ty-var.stderr @@ -0,0 +1,16 @@ +error[E0117]: only traits defined in the current crate can be implemented for arbitrary types + --> $DIR/orphan-check-error-reporting-ty-var.rs:14:1 + | +LL | impl From> for as MyTrait>::Item {} + | ^^^^^^^^^-------------^^^^^-------------------------- + | | | + | | `Vec` is not defined in the current crate + | `Vec` is not defined in the current crate + | + = note: impl doesn't have any local type before any uncovered type parameters + = note: for more information see https://doc.rust-lang.org/reference/items/implementations.html#orphan-rules + = note: define and implement a trait or new type instead + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0117`. From b14de91c5e7e60b946e242ec4dd299214711bae6 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 18 Mar 2025 21:42:19 +0000 Subject: [PATCH 12/16] Pre cleanups --- compiler/rustc_hir_typeck/src/closure.rs | 6 +---- .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 27 ++++++++++++------- .../rustc_hir_typeck/src/typeck_root_ctxt.rs | 2 +- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/closure.rs b/compiler/rustc_hir_typeck/src/closure.rs index fc1f9a7f2e0..a4776338f6c 100644 --- a/compiler/rustc_hir_typeck/src/closure.rs +++ b/compiler/rustc_hir_typeck/src/closure.rs @@ -164,11 +164,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let resume_ty = liberated_sig.inputs().get(0).copied().unwrap_or(tcx.types.unit); let interior = self.next_ty_var(expr_span); - self.deferred_coroutine_interiors.borrow_mut().push(( - expr_def_id, - body.id(), - interior, - )); + self.deferred_coroutine_interiors.borrow_mut().push((expr_def_id, interior)); // Coroutines that come from coroutine closures have not yet determined // their kind ty, so make a fresh infer var which will be constrained diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 64886957ff3..d75c2853ba0 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -633,18 +633,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let coroutines = std::mem::take(&mut *self.deferred_coroutine_interiors.borrow_mut()); debug!(?coroutines); - for &(expr_def_id, body_id, interior) in coroutines.iter() { - debug!(?expr_def_id); + let mut obligations = vec![]; + + for &(coroutine_def_id, interior) in coroutines.iter() { + debug!(?coroutine_def_id); // Create the `CoroutineWitness` type that we will unify with `interior`. let args = ty::GenericArgs::identity_for_item( self.tcx, - self.tcx.typeck_root_def_id(expr_def_id.to_def_id()), + self.tcx.typeck_root_def_id(coroutine_def_id.to_def_id()), ); - let witness = Ty::new_coroutine_witness(self.tcx, expr_def_id.to_def_id(), args); + let witness = Ty::new_coroutine_witness(self.tcx, coroutine_def_id.to_def_id(), args); // Unify `interior` with `witness` and collect all the resulting obligations. - let span = self.tcx.hir_body(body_id).value.span; + let span = self.tcx.hir_body_owned_by(coroutine_def_id).value.span; let ty::Infer(ty::InferTy::TyVar(_)) = interior.kind() else { span_bug!(span, "coroutine interior witness not infer: {:?}", interior.kind()) }; @@ -653,15 +655,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Will never define opaque types, as all we do is instantiate a type variable. .eq(DefineOpaqueTypes::Yes, interior, witness) .expect("Failed to unify coroutine interior type"); - let mut obligations = ok.obligations; - // Also collect the obligations that were unstalled by this unification. + obligations.extend(ok.obligations); + } + + // FIXME: Use a real visitor for unstalled obligations in the new solver. + if !coroutines.is_empty() { obligations .extend(self.fulfillment_cx.borrow_mut().drain_unstalled_obligations(&self.infcx)); - - let obligations = obligations.into_iter().map(|o| (o.predicate, o.cause)); - self.typeck_results.borrow_mut().coroutine_stalled_predicates.extend(obligations); } + + self.typeck_results + .borrow_mut() + .coroutine_stalled_predicates + .extend(obligations.into_iter().map(|o| (o.predicate, o.cause))); } #[instrument(skip(self), level = "debug")] diff --git a/compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs b/compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs index 88f4f7caa95..5b4fc51cec8 100644 --- a/compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs +++ b/compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs @@ -59,7 +59,7 @@ pub(crate) struct TypeckRootCtxt<'tcx> { pub(super) deferred_asm_checks: RefCell, HirId)>>, - pub(super) deferred_coroutine_interiors: RefCell)>>, + pub(super) deferred_coroutine_interiors: RefCell)>>, pub(super) deferred_repeat_expr_checks: RefCell, Ty<'tcx>, ty::Const<'tcx>)>>, From 2e36990881e25986ca5dc08b74d79b9a527b4404 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 20 Mar 2025 12:26:28 +1100 Subject: [PATCH 13/16] coverage: Convert and check span coordinates without a local file ID For expansion region support, we will want to be able to convert and check spans before creating a corresponding local file ID. If we create local file IDs eagerly, but some expansion turns out to have no successfully-converted spans, LLVM will complain about that expansion's file ID having no regions. --- .../src/coverageinfo/mapgen/covfun.rs | 22 ++++----- .../src/coverageinfo/mapgen/spans.rs | 45 +++++++++++++------ 2 files changed, 40 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index b8082edb9d5..048e1988c32 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -120,9 +120,14 @@ fn fill_region_tables<'tcx>( // Associate that global file ID with a local file ID for this function. let local_file_id = covfun.virtual_file_mapping.local_id_for_global(global_file_id); - let make_cov_span = - |span: Span| spans::make_coverage_span(local_file_id, source_map, &source_file, span); + // In rare cases, _all_ of a function's spans are discarded, and coverage + // codegen needs to handle that gracefully to avoid #133606. + // It's hard for tests to trigger this organically, so instead we set + // `-Zcoverage-options=discard-all-spans-in-codegen` to force it to occur. let discard_all = tcx.sess.coverage_discard_all_spans_in_codegen(); + let make_coords = |span: Span| { + if discard_all { None } else { spans::make_coords(source_map, &source_file, span) } + }; let ffi::Regions { code_regions, @@ -145,17 +150,8 @@ fn fill_region_tables<'tcx>( ffi::Counter::from_term(term) }; - // Convert the `Span` into coordinates that we can pass to LLVM, or - // discard the span if conversion fails. In rare, cases _all_ of a - // function's spans are discarded, and the rest of coverage codegen - // needs to handle that gracefully to avoid a repeat of #133606. - // We don't have a good test case for triggering that organically, so - // instead we set `-Zcoverage-options=discard-all-spans-in-codegen` - // to force it to occur. - let Some(cov_span) = make_cov_span(span) else { continue }; - if discard_all { - continue; - } + let Some(coords) = make_coords(span) else { continue }; + let cov_span = coords.make_coverage_span(local_file_id); match *kind { MappingKind::Code { bcb } => { diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs index 3193be31ada..39a59560c9d 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs @@ -5,22 +5,40 @@ use tracing::debug; use crate::coverageinfo::ffi; use crate::coverageinfo::mapgen::LocalFileId; +/// Line and byte-column coordinates of a source code span within some file. +/// The file itself must be tracked separately. +#[derive(Clone, Copy, Debug)] +pub(crate) struct Coords { + /// 1-based starting line of the source code span. + pub(crate) start_line: u32, + /// 1-based starting column (in bytes) of the source code span. + pub(crate) start_col: u32, + /// 1-based ending line of the source code span. + pub(crate) end_line: u32, + /// 1-based ending column (in bytes) of the source code span. High bit must be unset. + pub(crate) end_col: u32, +} + +impl Coords { + /// Attaches a local file ID to these coordinates to produce an `ffi::CoverageSpan`. + pub(crate) fn make_coverage_span(&self, local_file_id: LocalFileId) -> ffi::CoverageSpan { + let &Self { start_line, start_col, end_line, end_col } = self; + let file_id = local_file_id.as_u32(); + ffi::CoverageSpan { file_id, start_line, start_col, end_line, end_col } + } +} + /// Converts the span into its start line and column, and end line and column. /// /// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by /// the compiler, these column numbers are denoted in **bytes**, because that's what /// LLVM's `llvm-cov` tool expects to see in coverage maps. /// -/// Returns `None` if the conversion failed for some reason. This shouldn't happen, +/// Returns `None` if the conversion failed for some reason. This should be uncommon, /// but it's hard to rule out entirely (especially in the presence of complex macros /// or other expansions), and if it does happen then skipping a span or function is /// better than an ICE or `llvm-cov` failure that the user might have no way to avoid. -pub(crate) fn make_coverage_span( - file_id: LocalFileId, - source_map: &SourceMap, - file: &SourceFile, - span: Span, -) -> Option { +pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span) -> Option { let span = ensure_non_empty_span(source_map, span)?; let lo = span.lo(); @@ -44,8 +62,7 @@ pub(crate) fn make_coverage_span( start_line = source_map.doctest_offset_line(&file.name, start_line); end_line = source_map.doctest_offset_line(&file.name, end_line); - check_coverage_span(ffi::CoverageSpan { - file_id: file_id.as_u32(), + check_coords(Coords { start_line: start_line as u32, start_col: start_col as u32, end_line: end_line as u32, @@ -80,8 +97,8 @@ fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option { /// it will immediately exit with a fatal error. To prevent that from happening, /// discard regions that are improperly ordered, or might be interpreted in a /// way that makes them improperly ordered. -fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option { - let ffi::CoverageSpan { file_id: _, start_line, start_col, end_line, end_col } = cov_span; +fn check_coords(coords: Coords) -> Option { + let Coords { start_line, start_col, end_line, end_col } = coords; // Line/column coordinates are supposed to be 1-based. If we ever emit // coordinates of 0, `llvm-cov` might misinterpret them. @@ -94,17 +111,17 @@ fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option let is_ordered = (start_line, start_col) <= (end_line, end_col); if all_nonzero && end_col_has_high_bit_unset && is_ordered { - Some(cov_span) + Some(coords) } else { debug!( - ?cov_span, + ?coords, ?all_nonzero, ?end_col_has_high_bit_unset, ?is_ordered, "Skipping source region that would be misinterpreted or rejected by LLVM" ); // If this happens in a debug build, ICE to make it easier to notice. - debug_assert!(false, "Improper source region: {cov_span:?}"); + debug_assert!(false, "Improper source region: {coords:?}"); None } } From e6004ccb50880f9667015eea2f3d90f30517abd1 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 20 Mar 2025 03:21:58 +0000 Subject: [PATCH 14/16] Use def_path_str for def id arg in UnsupportedOpInfo --- compiler/rustc_const_eval/messages.ftl | 4 ++-- compiler/rustc_const_eval/src/errors.rs | 7 ++++--- tests/ui/consts/miri_unleashed/extern-static.stderr | 4 ++-- tests/ui/consts/miri_unleashed/tls.stderr | 4 ++-- tests/ui/extern/issue-28324.stderr | 2 +- tests/ui/statics/issue-14227.stderr | 2 +- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_const_eval/messages.ftl b/compiler/rustc_const_eval/messages.ftl index ccf9b240d40..dd481e04abb 100644 --- a/compiler/rustc_const_eval/messages.ftl +++ b/compiler/rustc_const_eval/messages.ftl @@ -93,7 +93,7 @@ const_eval_expected_inbounds_pointer = } const_eval_extern_static = - cannot access extern static ({$did}) + cannot access extern static `{$did}` const_eval_extern_type_field = `extern type` field does not have a known offset const_eval_fn_ptr_call = @@ -381,7 +381,7 @@ const_eval_thread_local_access = thread-local statics cannot be accessed at compile-time const_eval_thread_local_static = - cannot access thread local static ({$did}) + cannot access thread local static `{$did}` const_eval_too_generic = encountered overly generic constant const_eval_too_many_caller_args = diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index b020eeccf71..e2675e2f4c9 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -898,6 +898,7 @@ impl ReportErrorExt for UnsupportedOpInfo { UnsupportedOpInfo::ExternStatic(_) => const_eval_extern_static, } } + fn add_args(self, diag: &mut Diag<'_, G>) { use UnsupportedOpInfo::*; @@ -917,9 +918,9 @@ impl ReportErrorExt for UnsupportedOpInfo { OverwritePartialPointer(ptr) | ReadPartialPointer(ptr) => { diag.arg("ptr", ptr); } - ThreadLocalStatic(did) | ExternStatic(did) => { - diag.arg("did", format!("{did:?}")); - } + ThreadLocalStatic(did) | ExternStatic(did) => rustc_middle::ty::tls::with(|tcx| { + diag.arg("did", tcx.def_path_str(did)); + }), } } } diff --git a/tests/ui/consts/miri_unleashed/extern-static.stderr b/tests/ui/consts/miri_unleashed/extern-static.stderr index 0979a5e4fb1..4dbabbe44a2 100644 --- a/tests/ui/consts/miri_unleashed/extern-static.stderr +++ b/tests/ui/consts/miri_unleashed/extern-static.stderr @@ -2,13 +2,13 @@ error[E0080]: could not evaluate static initializer --> $DIR/extern-static.rs:11:25 | LL | unsafe { let _val = DATA; } - | ^^^^ cannot access extern static (DefId(0:4 ~ extern_static[c41e]::{extern#0}::DATA)) + | ^^^^ cannot access extern static `DATA` error[E0080]: could not evaluate static initializer --> $DIR/extern-static.rs:16:14 | LL | unsafe { DATA = 0; } - | ^^^^^^^^ cannot access extern static (DefId(0:4 ~ extern_static[c41e]::{extern#0}::DATA)) + | ^^^^^^^^ cannot access extern static `DATA` error: aborting due to 2 previous errors diff --git a/tests/ui/consts/miri_unleashed/tls.stderr b/tests/ui/consts/miri_unleashed/tls.stderr index a00b7eb1312..ef836544303 100644 --- a/tests/ui/consts/miri_unleashed/tls.stderr +++ b/tests/ui/consts/miri_unleashed/tls.stderr @@ -2,13 +2,13 @@ error[E0080]: could not evaluate static initializer --> $DIR/tls.rs:11:25 | LL | unsafe { let _val = A; } - | ^ cannot access thread local static (DefId(0:4 ~ tls[ca29]::A)) + | ^ cannot access thread local static `A` error[E0080]: could not evaluate static initializer --> $DIR/tls.rs:20:26 | LL | unsafe { let _val = &A; } - | ^ cannot access thread local static (DefId(0:4 ~ tls[ca29]::A)) + | ^ cannot access thread local static `A` warning: skipping const checks | diff --git a/tests/ui/extern/issue-28324.stderr b/tests/ui/extern/issue-28324.stderr index 1fccb34fdf3..93eb6ff8174 100644 --- a/tests/ui/extern/issue-28324.stderr +++ b/tests/ui/extern/issue-28324.stderr @@ -2,7 +2,7 @@ error[E0080]: could not evaluate static initializer --> $DIR/issue-28324.rs:5:23 | LL | pub static BAZ: u32 = *&error_message_count; - | ^^^^^^^^^^^^^^^^^^^^^ cannot access extern static (DefId(0:4 ~ issue_28324[8ec4]::{extern#0}::error_message_count)) + | ^^^^^^^^^^^^^^^^^^^^^ cannot access extern static `error_message_count` error[E0133]: use of extern static is unsafe and requires unsafe function or block --> $DIR/issue-28324.rs:5:25 diff --git a/tests/ui/statics/issue-14227.stderr b/tests/ui/statics/issue-14227.stderr index 0aeb973bff3..3551821a3da 100644 --- a/tests/ui/statics/issue-14227.stderr +++ b/tests/ui/statics/issue-14227.stderr @@ -2,7 +2,7 @@ error[E0080]: could not evaluate static initializer --> $DIR/issue-14227.rs:4:21 | LL | static CRASH: u32 = symbol; - | ^^^^^^ cannot access extern static (DefId(0:4 ~ issue_14227[1133]::{extern#0}::symbol)) + | ^^^^^^ cannot access extern static `symbol` error[E0133]: use of extern static is unsafe and requires unsafe function or block --> $DIR/issue-14227.rs:4:21 From 974f7590e6ab16dce66c646a4e225c2d041d2b05 Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Thu, 20 Mar 2025 12:37:24 +0800 Subject: [PATCH 15/16] Remove `llvm` and `llvms` triagebot ping aliases for icebreakers-llvm Because it's way too easy to confuse that versus trying to ping WG-llvm. And AFAIK, icebreakers-llvm isn't really used in a good while. --- triagebot.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/triagebot.toml b/triagebot.toml index cd488a7cdf6..53cdd8b585b 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -47,7 +47,6 @@ add_labels = ["S-waiting-on-review"] [glacier] [ping.icebreakers-llvm] -alias = ["llvm", "llvms"] message = """\ Hey LLVM ICE-breakers! This bug has been identified as a good "LLVM ICE-breaking candidate". In case it's useful, here are some From 496c251d176ee381bf29a183c4cf330243f52c1f Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Thu, 20 Mar 2025 12:40:51 +0800 Subject: [PATCH 16/16] Disambiguate between wg-llvm and icebreakers-llvm in rustc-dev-guide --- .../src/notification-groups/about.md | 4 ++-- .../rustc-dev-guide/src/notification-groups/llvm.md | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/doc/rustc-dev-guide/src/notification-groups/about.md b/src/doc/rustc-dev-guide/src/notification-groups/about.md index 74629aa08ac..af305f0103a 100644 --- a/src/doc/rustc-dev-guide/src/notification-groups/about.md +++ b/src/doc/rustc-dev-guide/src/notification-groups/about.md @@ -23,7 +23,7 @@ Here's the list of the notification groups: - [ARM](./arm.md) - [Cleanup Crew](./cleanup-crew.md) - [Emscripten](./emscripten.md) -- [LLVM](./llvm.md) +- [LLVM Icebreakers](./llvm.md) - [RISC-V](./risc-v.md) - [WASI](./wasi.md) - [WebAssembly](./wasm.md) @@ -83,7 +83,7 @@ group. For example: @rustbot ping arm @rustbot ping cleanup-crew @rustbot ping emscripten -@rustbot ping llvm +@rustbot ping icebreakers-llvm @rustbot ping risc-v @rustbot ping wasi @rustbot ping wasm diff --git a/src/doc/rustc-dev-guide/src/notification-groups/llvm.md b/src/doc/rustc-dev-guide/src/notification-groups/llvm.md index 2eff63713a9..9d008728543 100644 --- a/src/doc/rustc-dev-guide/src/notification-groups/llvm.md +++ b/src/doc/rustc-dev-guide/src/notification-groups/llvm.md @@ -1,13 +1,16 @@ -# LLVM Notification group +# LLVM Icebreakers Notification group **Github Label:** [A-LLVM]
-**Ping command:** `@rustbot ping llvm` +**Ping command:** `@rustbot ping icebreakers-llvm` [A-LLVM]: https://github.com/rust-lang/rust/labels/A-LLVM -The "LLVM Notification Group" are focused on bugs that center around LLVM. -These bugs often arise because of LLVM optimizations gone awry, or as -the result of an LLVM upgrade. The goal here is: +*Note*: this notification group is *not* the same as the LLVM working group +(WG-llvm). + +The "LLVM Icebreakers Notification Group" are focused on bugs that center around +LLVM. These bugs often arise because of LLVM optimizations gone awry, or as the +result of an LLVM upgrade. The goal here is: - to determine whether the bug is a result of us generating invalid LLVM IR, or LLVM misoptimizing;