From c7a67747d3320b37f87241cd4b8fe4ca014d328e Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Thu, 28 Dec 2023 21:19:41 +0100 Subject: [PATCH 1/5] Use filter instead of filter_map in Parser::expected_one_of_not_found --- .../rustc_parse/src/parser/diagnostics.rs | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index b4493df57e2ac..77bca2f138a9d 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -450,37 +450,39 @@ impl<'a> Parser<'a> { let mut expected = edible .iter() - .map(|x| TokenType::Token(x.clone())) - .chain(inedible.iter().map(|x| TokenType::Token(x.clone()))) + .chain(inedible) + .cloned() + .map(TokenType::Token) .chain(self.expected_tokens.iter().cloned()) - .filter_map(|token| { - // filter out suggestions which suggest the same token which was found and deemed incorrect + .filter(|token| { + // Filter out suggestions that suggest the same token which was found and deemed incorrect. fn is_ident_eq_keyword(found: &TokenKind, expected: &TokenType) -> bool { - if let TokenKind::Ident(current_sym, _) = found { - if let TokenType::Keyword(suggested_sym) = expected { - return current_sym == suggested_sym; - } + if let TokenKind::Ident(current_sym, _) = found + && let TokenType::Keyword(suggested_sym) = expected + { + return current_sym == suggested_sym; } false } - if token != parser::TokenType::Token(self.token.kind.clone()) { + + if *token != parser::TokenType::Token(self.token.kind.clone()) { let eq = is_ident_eq_keyword(&self.token.kind, &token); - // if the suggestion is a keyword and the found token is an ident, + // If the suggestion is a keyword and the found token is an ident, // the content of which are equal to the suggestion's content, - // we can remove that suggestion (see the return None statement below) + // we can remove that suggestion (see the `return false` below). - // if this isn't the case however, and the suggestion is a token the - // content of which is the same as the found token's, we remove it as well + // If this isn't the case however, and the suggestion is a token the + // content of which is the same as the found token's, we remove it as well. if !eq { if let TokenType::Token(kind) = &token { if kind == &self.token.kind { - return None; + return false; } } - return Some(token); + return true; } } - return None; + false }) .collect::>(); expected.sort_by_cached_key(|x| x.to_string()); @@ -488,10 +490,10 @@ impl<'a> Parser<'a> { let sm = self.sess.source_map(); - // Special-case "expected `;`" errors + // Special-case "expected `;`" errors. if expected.contains(&TokenType::Token(token::Semi)) { // If the user is trying to write a ternary expression, recover it and - // return an Err to prevent a cascade of irrelevant diagnostics + // return an Err to prevent a cascade of irrelevant diagnostics. if self.prev_token == token::Question && let Err(e) = self.maybe_recover_from_ternary_operator() { From 2480a0f3f69fbca417990d68d816ed5423ad2cd6 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Thu, 28 Dec 2023 13:57:43 +0100 Subject: [PATCH 2/5] Merge Coroutine lowering functions Instead of having separate `make_async/etc_expr` functions, this merges them them into one, reducing code duplication a bit. --- compiler/rustc_ast_lowering/src/expr.rs | 266 ++++++------------------ compiler/rustc_ast_lowering/src/item.rs | 39 ++-- 2 files changed, 81 insertions(+), 224 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index ccc6644923ae6..e568da9bbc0c2 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -183,14 +183,6 @@ impl<'hir> LoweringContext<'_, 'hir> { self.arena.alloc_from_iter(arms.iter().map(|x| self.lower_arm(x))), hir::MatchSource::Normal, ), - ExprKind::Gen(capture_clause, block, GenBlockKind::Async) => self.make_async_expr( - *capture_clause, - e.id, - None, - e.span, - hir::CoroutineSource::Block, - |this| this.with_new_scopes(e.span, |this| this.lower_block_expr(block)), - ), ExprKind::Await(expr, await_kw_span) => self.lower_expr_await(*await_kw_span, expr), ExprKind::Closure(box Closure { binder, @@ -226,6 +218,22 @@ impl<'hir> LoweringContext<'_, 'hir> { *fn_arg_span, ), }, + ExprKind::Gen(capture_clause, block, genblock_kind) => { + let desugaring_kind = match genblock_kind { + GenBlockKind::Async => hir::CoroutineDesugaring::Async, + GenBlockKind::Gen => hir::CoroutineDesugaring::Gen, + GenBlockKind::AsyncGen => hir::CoroutineDesugaring::AsyncGen, + }; + self.make_desugared_coroutine_expr( + *capture_clause, + e.id, + None, + e.span, + desugaring_kind, + hir::CoroutineSource::Block, + |this| this.with_new_scopes(e.span, |this| this.lower_block_expr(block)), + ) + } ExprKind::Block(blk, opt_label) => { let opt_label = self.lower_label(*opt_label); hir::ExprKind::Block(self.lower_block(blk, opt_label.is_some()), opt_label) @@ -313,23 +321,6 @@ impl<'hir> LoweringContext<'_, 'hir> { rest, ) } - ExprKind::Gen(capture_clause, block, GenBlockKind::Gen) => self.make_gen_expr( - *capture_clause, - e.id, - None, - e.span, - hir::CoroutineSource::Block, - |this| this.with_new_scopes(e.span, |this| this.lower_block_expr(block)), - ), - ExprKind::Gen(capture_clause, block, GenBlockKind::AsyncGen) => self - .make_async_gen_expr( - *capture_clause, - e.id, - None, - e.span, - hir::CoroutineSource::Block, - |this| this.with_new_scopes(e.span, |this| this.lower_block_expr(block)), - ), ExprKind::Yield(opt_expr) => self.lower_expr_yield(e.span, opt_expr.as_deref()), ExprKind::Err => { hir::ExprKind::Err(self.dcx().span_delayed_bug(e.span, "lowered ExprKind::Err")) @@ -612,213 +603,91 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::Arm { hir_id, pat, guard, body, span } } - /// Lower an `async` construct to a coroutine that implements `Future`. + /// Lower/desugar a coroutine construct. /// - /// This results in: - /// - /// ```text - /// static move? |_task_context| -> { - /// - /// } - /// ``` - pub(super) fn make_async_expr( - &mut self, - capture_clause: CaptureBy, - closure_node_id: NodeId, - ret_ty: Option>, - span: Span, - async_coroutine_source: hir::CoroutineSource, - body: impl FnOnce(&mut Self) -> hir::Expr<'hir>, - ) -> hir::ExprKind<'hir> { - let output = ret_ty.unwrap_or_else(|| hir::FnRetTy::DefaultReturn(self.lower_span(span))); - - // Resume argument type: `ResumeTy` - let unstable_span = self.mark_span_with_reason( - DesugaringKind::Async, - self.lower_span(span), - Some(self.allow_gen_future.clone()), - ); - let resume_ty = self.make_lang_item_qpath(hir::LangItem::ResumeTy, unstable_span); - let input_ty = hir::Ty { - hir_id: self.next_id(), - kind: hir::TyKind::Path(resume_ty), - span: unstable_span, - }; - - // The closure/coroutine `FnDecl` takes a single (resume) argument of type `input_ty`. - let fn_decl = self.arena.alloc(hir::FnDecl { - inputs: arena_vec![self; input_ty], - output, - c_variadic: false, - implicit_self: hir::ImplicitSelfKind::None, - lifetime_elision_allowed: false, - }); - - // Lower the argument pattern/ident. The ident is used again in the `.await` lowering. - let (pat, task_context_hid) = self.pat_ident_binding_mode( - span, - Ident::with_dummy_span(sym::_task_context), - hir::BindingAnnotation::MUT, - ); - let param = hir::Param { - hir_id: self.next_id(), - pat, - ty_span: self.lower_span(span), - span: self.lower_span(span), - }; - let params = arena_vec![self; param]; - - let coroutine_kind = - hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Async, async_coroutine_source); - let body = self.lower_body(move |this| { - this.coroutine_kind = Some(coroutine_kind); - - let old_ctx = this.task_context; - this.task_context = Some(task_context_hid); - let res = body(this); - this.task_context = old_ctx; - (params, res) - }); - - // `static |_task_context| -> { body }`: - hir::ExprKind::Closure(self.arena.alloc(hir::Closure { - def_id: self.local_def_id(closure_node_id), - binder: hir::ClosureBinder::Default, - capture_clause, - bound_generic_params: &[], - fn_decl, - body, - fn_decl_span: self.lower_span(span), - fn_arg_span: None, - kind: hir::ClosureKind::Coroutine(coroutine_kind), - constness: hir::Constness::NotConst, - })) - } - - /// Lower a `gen` construct to a generator that implements `Iterator`. + /// In particular, this creates the correct async resume argument and `_task_context`. /// /// This results in: /// /// ```text - /// static move? |()| -> () { + /// static move? |<_task_context?>| -> { /// /// } /// ``` - pub(super) fn make_gen_expr( + pub(super) fn make_desugared_coroutine_expr( &mut self, capture_clause: CaptureBy, closure_node_id: NodeId, - _yield_ty: Option>, + return_ty: Option>, span: Span, + desugaring_kind: hir::CoroutineDesugaring, coroutine_source: hir::CoroutineSource, body: impl FnOnce(&mut Self) -> hir::Expr<'hir>, ) -> hir::ExprKind<'hir> { - let output = hir::FnRetTy::DefaultReturn(self.lower_span(span)); - - // The closure/generator `FnDecl` takes a single (resume) argument of type `input_ty`. - let fn_decl = self.arena.alloc(hir::FnDecl { - inputs: &[], - output, - c_variadic: false, - implicit_self: hir::ImplicitSelfKind::None, - lifetime_elision_allowed: false, - }); - - let coroutine_kind = - hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Gen, coroutine_source); - let body = self.lower_body(move |this| { - this.coroutine_kind = Some(coroutine_kind); - - let res = body(this); - (&[], res) - }); - - // `static |()| -> () { body }`: - hir::ExprKind::Closure(self.arena.alloc(hir::Closure { - def_id: self.local_def_id(closure_node_id), - binder: hir::ClosureBinder::Default, - capture_clause, - bound_generic_params: &[], - fn_decl, - body, - fn_decl_span: self.lower_span(span), - fn_arg_span: None, - kind: hir::ClosureKind::Coroutine(coroutine_kind), - constness: hir::Constness::NotConst, - })) - } + let coroutine_kind = hir::CoroutineKind::Desugared(desugaring_kind, coroutine_source); + + // The `async` desugaring takes a resume argument and maintains a `task_context`, + // whereas a generator does not. + let (inputs, params, task_context): (&[_], &[_], _) = match desugaring_kind { + hir::CoroutineDesugaring::Async | hir::CoroutineDesugaring::AsyncGen => { + // Resume argument type: `ResumeTy` + let unstable_span = self.mark_span_with_reason( + DesugaringKind::Async, + self.lower_span(span), + Some(self.allow_gen_future.clone()), + ); + let resume_ty = self.make_lang_item_qpath(hir::LangItem::ResumeTy, unstable_span); + let input_ty = hir::Ty { + hir_id: self.next_id(), + kind: hir::TyKind::Path(resume_ty), + span: unstable_span, + }; + let inputs = arena_vec![self; input_ty]; - /// Lower a `async gen` construct to a generator that implements `AsyncIterator`. - /// - /// This results in: - /// - /// ```text - /// static move? |_task_context| -> () { - /// - /// } - /// ``` - pub(super) fn make_async_gen_expr( - &mut self, - capture_clause: CaptureBy, - closure_node_id: NodeId, - _yield_ty: Option>, - span: Span, - async_coroutine_source: hir::CoroutineSource, - body: impl FnOnce(&mut Self) -> hir::Expr<'hir>, - ) -> hir::ExprKind<'hir> { - let output = hir::FnRetTy::DefaultReturn(self.lower_span(span)); + // Lower the argument pattern/ident. The ident is used again in the `.await` lowering. + let (pat, task_context_hid) = self.pat_ident_binding_mode( + span, + Ident::with_dummy_span(sym::_task_context), + hir::BindingAnnotation::MUT, + ); + let param = hir::Param { + hir_id: self.next_id(), + pat, + ty_span: self.lower_span(span), + span: self.lower_span(span), + }; + let params = arena_vec![self; param]; - // Resume argument type: `ResumeTy` - let unstable_span = self.mark_span_with_reason( - DesugaringKind::Async, - self.lower_span(span), - Some(self.allow_gen_future.clone()), - ); - let resume_ty = self.make_lang_item_qpath(hir::LangItem::ResumeTy, unstable_span); - let input_ty = hir::Ty { - hir_id: self.next_id(), - kind: hir::TyKind::Path(resume_ty), - span: unstable_span, + (inputs, params, Some(task_context_hid)) + } + hir::CoroutineDesugaring::Gen => (&[], &[], None), }; - // The closure/coroutine `FnDecl` takes a single (resume) argument of type `input_ty`. + let output = + return_ty.unwrap_or_else(|| hir::FnRetTy::DefaultReturn(self.lower_span(span))); + let fn_decl = self.arena.alloc(hir::FnDecl { - inputs: arena_vec![self; input_ty], + inputs, output, c_variadic: false, implicit_self: hir::ImplicitSelfKind::None, lifetime_elision_allowed: false, }); - // Lower the argument pattern/ident. The ident is used again in the `.await` lowering. - let (pat, task_context_hid) = self.pat_ident_binding_mode( - span, - Ident::with_dummy_span(sym::_task_context), - hir::BindingAnnotation::MUT, - ); - let param = hir::Param { - hir_id: self.next_id(), - pat, - ty_span: self.lower_span(span), - span: self.lower_span(span), - }; - let params = arena_vec![self; param]; - - let coroutine_kind = hir::CoroutineKind::Desugared( - hir::CoroutineDesugaring::AsyncGen, - async_coroutine_source, - ); let body = self.lower_body(move |this| { this.coroutine_kind = Some(coroutine_kind); let old_ctx = this.task_context; - this.task_context = Some(task_context_hid); + if task_context.is_some() { + this.task_context = task_context; + } let res = body(this); this.task_context = old_ctx; + (params, res) }); - // `static |_task_context| -> { body }`: + // `static |<_task_context?>| -> { }`: hir::ExprKind::Closure(self.arena.alloc(hir::Closure { def_id: self.local_def_id(closure_node_id), binder: hir::ClosureBinder::Default, @@ -1203,11 +1072,12 @@ impl<'hir> LoweringContext<'_, 'hir> { None }; - let async_body = this.make_async_expr( + let async_body = this.make_desugared_coroutine_expr( capture_clause, inner_closure_id, async_ret_ty, body.span, + hir::CoroutineDesugaring::Async, hir::CoroutineSource::Closure, |this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)), ); diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 3848f3b778298..45357aca53390 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -1209,33 +1209,20 @@ impl<'hir> LoweringContext<'_, 'hir> { this.expr_block(body) }; - // FIXME(gen_blocks): Consider unifying the `make_*_expr` functions. - let coroutine_expr = match coroutine_kind { - CoroutineKind::Async { .. } => this.make_async_expr( - CaptureBy::Value { move_kw: rustc_span::DUMMY_SP }, - closure_id, - None, - body.span, - hir::CoroutineSource::Fn, - mkbody, - ), - CoroutineKind::Gen { .. } => this.make_gen_expr( - CaptureBy::Value { move_kw: rustc_span::DUMMY_SP }, - closure_id, - None, - body.span, - hir::CoroutineSource::Fn, - mkbody, - ), - CoroutineKind::AsyncGen { .. } => this.make_async_gen_expr( - CaptureBy::Value { move_kw: rustc_span::DUMMY_SP }, - closure_id, - None, - body.span, - hir::CoroutineSource::Fn, - mkbody, - ), + let desugaring_kind = match coroutine_kind { + CoroutineKind::Async { .. } => hir::CoroutineDesugaring::Async, + CoroutineKind::Gen { .. } => hir::CoroutineDesugaring::Gen, + CoroutineKind::AsyncGen { .. } => hir::CoroutineDesugaring::AsyncGen, }; + let coroutine_expr = this.make_desugared_coroutine_expr( + CaptureBy::Value { move_kw: rustc_span::DUMMY_SP }, + closure_id, + None, + body.span, + desugaring_kind, + hir::CoroutineSource::Fn, + mkbody, + ); let hir_id = this.lower_node_id(closure_id); this.maybe_forward_track_caller(body.span, fn_id, hir_id); From 8529b63e2bc1afd6cefdf204e6b576b98394371c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 29 Dec 2023 12:00:48 +1100 Subject: [PATCH 3/5] coverage: Avoid a possible query stability hazard in `CoverageCounters` The iteration order of this hashmap can potentially affect the relative creation order of MIR blocks. --- compiler/rustc_mir_transform/src/coverage/counters.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 604589e5b96ba..d995d562521da 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -1,4 +1,4 @@ -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::graph::WithNumNodes; use rustc_index::bit_set::BitSet; use rustc_index::IndexVec; @@ -47,7 +47,10 @@ pub(super) struct CoverageCounters { bcb_counters: IndexVec>, /// Coverage counters/expressions that are associated with the control-flow /// edge between two BCBs. - bcb_edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>, + /// + /// The iteration order of this map can affect the precise contents of MIR, + /// so we use `FxIndexMap` to avoid query stability hazards. + bcb_edge_counters: FxIndexMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>, /// Tracks which BCBs have a counter associated with some incoming edge. /// Only used by assertions, to verify that BCBs with incoming edge /// counters do not have their own physical counters (expressions are allowed). @@ -64,7 +67,7 @@ impl CoverageCounters { Self { next_counter_id: CounterId::START, bcb_counters: IndexVec::from_elem_n(None, num_bcbs), - bcb_edge_counters: FxHashMap::default(), + bcb_edge_counters: FxIndexMap::default(), bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs), expressions: IndexVec::new(), } From ab60a7df6446bf175b9717ff4e96288287393c02 Mon Sep 17 00:00:00 2001 From: est31 Date: Fri, 29 Dec 2023 02:52:11 +0100 Subject: [PATCH 4/5] Also walk bindings created by if-let guards --- compiler/rustc_passes/src/liveness.rs | 3 +++ tests/ui/lint/unused/issue-119383.rs | 9 +++++++++ tests/ui/lint/unused/issue-119383.stderr | 14 ++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 tests/ui/lint/unused/issue-119383.rs create mode 100644 tests/ui/lint/unused/issue-119383.stderr diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index 6be01e0d88fca..832c0c6b3174f 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -1351,6 +1351,9 @@ impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> { fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) { self.check_unused_vars_in_pat(arm.pat, None, None, |_, _, _, _| {}); + if let Some(hir::Guard::IfLet(let_expr)) = arm.guard { + self.check_unused_vars_in_pat(let_expr.pat, None, None, |_, _, _, _| {}); + } intravisit::walk_arm(self, arm); } } diff --git a/tests/ui/lint/unused/issue-119383.rs b/tests/ui/lint/unused/issue-119383.rs new file mode 100644 index 0000000000000..71197444f45c3 --- /dev/null +++ b/tests/ui/lint/unused/issue-119383.rs @@ -0,0 +1,9 @@ +#![feature(if_let_guard)] +#![deny(unused_variables)] + +fn main() { + match () { + () if let Some(b) = Some(()) => {} //~ ERROR unused variable: `b` + _ => {} + } +} diff --git a/tests/ui/lint/unused/issue-119383.stderr b/tests/ui/lint/unused/issue-119383.stderr new file mode 100644 index 0000000000000..6b4d14b95d55a --- /dev/null +++ b/tests/ui/lint/unused/issue-119383.stderr @@ -0,0 +1,14 @@ +error: unused variable: `b` + --> $DIR/issue-119383.rs:6:24 + | +LL | () if let Some(b) = Some(()) => {} + | ^ help: if this is intentional, prefix it with an underscore: `_b` + | +note: the lint level is defined here + --> $DIR/issue-119383.rs:2:9 + | +LL | #![deny(unused_variables)] + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + From f7ed423f50bf78e15d4a82a4899c4f084a61b2df Mon Sep 17 00:00:00 2001 From: Qiu Chaofan Date: Fri, 29 Dec 2023 16:50:24 +0800 Subject: [PATCH 5/5] Enable profiler in dist-powerpc-linux --- src/ci/docker/host-x86_64/dist-powerpc-linux/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/dist-powerpc-linux/Dockerfile b/src/ci/docker/host-x86_64/dist-powerpc-linux/Dockerfile index b546f571f66bd..7081d9527f060 100644 --- a/src/ci/docker/host-x86_64/dist-powerpc-linux/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-powerpc-linux/Dockerfile @@ -26,5 +26,5 @@ ENV \ ENV HOSTS=powerpc-unknown-linux-gnu -ENV RUST_CONFIGURE_ARGS --enable-extended --disable-docs +ENV RUST_CONFIGURE_ARGS --enable-extended --enable-profiler --disable-docs ENV SCRIPT python3 ../x.py dist --host $HOSTS --target $HOSTS