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 9b2d562a69ecd..1df6283af2647 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 0000000000000..77b1c2a19d0a4 --- /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 1b831c454e6d5..9dcdd86834372 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 4a1636e6aec0e..5a9df6ffadf03 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 e43d78f6e7217..116847af9f497 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 cfcb0e23cb5e6..a72e834e17a2f 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 5bb6704dde45a..19a08f92ce75c 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 faa47274f96ce..2bbf957feebe8 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 e3c41f117abb8..9e53ed41c46c4 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 a949ab94f3ad7..54c3cebedfc0e 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 1df6283af2647..cb04bbc240e49 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 fe716c186389f..bee391532294d 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 0000000000000..f76e8142ae86c --- /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 77b1c2a19d0a4..ff843138c8c2c 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 707312a97f3bc..deda6030831ec 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 eff2d2e3ff4a3..92c1ffa6076d3 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 0000000000000..8a8958f3ad4e8 --- /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 0000000000000..7e94e1e095add --- /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 0d809c17989de..206912d8de40d 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 cb04bbc240e49..28d100074f3eb 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 fd2ef9cb1db9d..fabb44005532d 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 bee391532294d..1811752c3c439 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 8a8958f3ad4e8..0000000000000 --- 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 7e94e1e095add..8ee34ec431226 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 9e53ed41c46c4..caba5f2721ab1 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 28d100074f3eb..15a625314b4a9 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 9dcdd86834372..5b7545b339663 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 5a9df6ffadf03..b159e136245da 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 116847af9f497..989ebe14bf8fc 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 a72e834e17a2f..ce8d6df75afb2 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 19a08f92ce75c..7f3d060bb8a37 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 2bbf957feebe8..faa47274f96ce 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 caba5f2721ab1..7d9dc89bd7567 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 54c3cebedfc0e..a949ab94f3ad7 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 15a625314b4a9..fc9a511d56a55 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 206912d8de40d..0d809c17989de 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 deda6030831ec..54261079fcad8 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 fabb44005532d..034ecde068a98 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 92c1ffa6076d3..e866f13efc73e 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 1811752c3c439..fcd475b1784f5 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 e24b45c5b1947..e08850da4a7a8 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 2694a1eda78d7..978186f76a1f0 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 208d510db2e1f..899e145c2c049 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 b5857e359a2ce..751c379b21a64 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 b79ae1e7cc21e..506358341b501 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 ca820deebdfad..84d07c711fa40 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 5fed2e352879c..170500c7a1628 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 98b81dd3def34..ddaca89ccf82d 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 90bdb3c4b3739..f4bd7ec701f8a 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 cdfae51583b60..908c3ee2eb8e3 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 715e3506ab824..752636ccaf061 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 e60904eebeb8b..24248e8dde34c 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 5536c93f84a43..dc453b1e747ca 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 2e589150d3ee6..f17efab81ec53 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 94a5a3769a322..527c18addbe12 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 bcfcc8000c718..3d666055a94fb 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 393d175ea4cd0..dc8022b95c313 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 4ecf702d7b6ef..de6dc088176ff 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 5ad83f886e2ec..041f6228fba2d 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 3dd2de1fafc72..8d47c756fc53c 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 701d06e4fd403..43b78423c727f 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 df0c3977dc8f6..4ea06a06539af 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 0000000000000..65b595635b158 --- /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 0000000000000..4f7ed8219521d --- /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 b617f4d37f5bf..f6000e7284002 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 2cd7fa3225acd..907d6d41a1fb5 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 5b487bc1a8bde..b8082edb9d532 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 39087a4d6f41d..83efb3ea66039 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 0471baa1f9cad..b8884486c3330 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 0b7fc44460ead..74ba4ffe25ea1 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 5329020360053..f8215a228f5b6 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 fab1a11304d57..c8c293121ca0a 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 9889cecdac541..0000000000000 --- 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 0000000000000..99a8345335448 --- /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 0000000000000..f229f8b2e385e --- /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 fc1f9a7f2e072..a4776338f6c39 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 64886957ff37f..d75c2853ba080 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 88f4f7caa95d8..5b4fc51cec885 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 b8082edb9d532..048e1988c3278 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 3193be31ada9f..39a59560c9d3e 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 ccf9b240d4088..dd481e04abbdc 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 b020eeccf717f..e2675e2f4c900 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 0979a5e4fb197..4dbabbe44a2bb 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 a00b7eb13128c..ef83654430318 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 1fccb34fdf37b..93eb6ff8174e9 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 0aeb973bff301..3551821a3dadb 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 cd488a7cdf65c..53cdd8b585b4a 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 74629aa08acec..af305f0103ae8 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 2eff63713a9c9..9d0087285438d 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;