From 15c01eb22c8f7abbfe6d49dc5824c239547a0323 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 18 Dec 2024 18:37:54 +0000 Subject: [PATCH 01/14] Fix cycle error only occurring with -Zdump-mir --- compiler/rustc_middle/src/mir/pretty.rs | 24 ++++++++++++------- tests/mir-opt/building/dump_mir_cycle.rs | 19 +++++++++++++++ ...0].SimplifyCfg-pre-optimizations.after.mir | 4 +--- ...motion_extern_static.BAR.PromoteTemps.diff | 6 ++--- 4 files changed, 37 insertions(+), 16 deletions(-) create mode 100644 tests/mir-opt/building/dump_mir_cycle.rs diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 47522f00bb1b3..24d2478f77018 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -1555,16 +1555,22 @@ pub fn write_allocations<'tcx>( write!(w, " (vtable: impl {dyn_ty} for {ty})")? } Some(GlobalAlloc::Static(did)) if !tcx.is_foreign_item(did) => { - match tcx.eval_static_initializer(did) { - Ok(alloc) => { - write!(w, " (static: {}, ", tcx.def_path_str(did))?; - write_allocation_track_relocs(w, alloc)?; + write!(w, " (static: {}", tcx.def_path_str(did))?; + if body.phase <= MirPhase::Runtime(RuntimePhase::PostCleanup) + && tcx.hir().body_const_context(body.source.def_id()).is_some() + { + // Statics may be cyclic and evaluating them too early + // in the MIR pipeline may cause cycle errors even though + // normal compilation is fine. + write!(w, ")")?; + } else { + match tcx.eval_static_initializer(did) { + Ok(alloc) => { + write!(w, ", ")?; + write_allocation_track_relocs(w, alloc)?; + } + Err(_) => write!(w, ", error during initializer evaluation)")?, } - Err(_) => write!( - w, - " (static: {}, error during initializer evaluation)", - tcx.def_path_str(did) - )?, } } Some(GlobalAlloc::Static(did)) => { diff --git a/tests/mir-opt/building/dump_mir_cycle.rs b/tests/mir-opt/building/dump_mir_cycle.rs new file mode 100644 index 0000000000000..8e13420aed790 --- /dev/null +++ b/tests/mir-opt/building/dump_mir_cycle.rs @@ -0,0 +1,19 @@ +#[derive(Debug)] +pub struct Thing { + pub next: &'static Thing, +} + +pub static THING: Thing = Thing { next: &THING }; +// CHECK: alloc{{.+}} (static: THING) + +const fn thing() -> &'static Thing { + &MUTUALLY_RECURSIVE +} + +pub static MUTUALLY_RECURSIVE: Thing = Thing { next: thing() }; +// CHECK: alloc{{.+}} (static: MUTUALLY_RECURSIVE) + +fn main() { + // Generate optimized MIR for the const fn, too. + thing(); +} diff --git a/tests/mir-opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-pre-optimizations.after.mir b/tests/mir-opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-pre-optimizations.after.mir index 7affbf6dd403f..344851bb08839 100644 --- a/tests/mir-opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-pre-optimizations.after.mir +++ b/tests/mir-opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-pre-optimizations.after.mir @@ -15,6 +15,4 @@ const BAR::promoted[0]: &[&i32; 1] = { } } -ALLOC0 (static: Y, size: 4, align: 4) { - 2a 00 00 00 │ *... -} +ALLOC0 (static: Y) diff --git a/tests/mir-opt/const_promotion_extern_static.BAR.PromoteTemps.diff b/tests/mir-opt/const_promotion_extern_static.BAR.PromoteTemps.diff index 487f68a8d4dfe..5f8f84244af6c 100644 --- a/tests/mir-opt/const_promotion_extern_static.BAR.PromoteTemps.diff +++ b/tests/mir-opt/const_promotion_extern_static.BAR.PromoteTemps.diff @@ -38,9 +38,7 @@ bb2 (cleanup): { resume; } -- } -- -- ALLOC0 (static: Y, size: 4, align: 4) { -- 2a 00 00 00 │ *... } +- +- ALLOC0 (static: Y) From 57f9f8f883b2621296549d82669299aecd00cc78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 31 Dec 2024 17:03:03 +0000 Subject: [PATCH 02/14] Add test for `mut arg: &Ty` meant to be `arg: &mut Ty` This is a mistake I've seen newcomers make where they want to express an "out" argument. --- ...owed-type-meant-to-be-arg-of-mut-borrow.rs | 20 +++++++ ...-type-meant-to-be-arg-of-mut-borrow.stderr | 60 +++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs create mode 100644 tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs new file mode 100644 index 0000000000000..eff4745e4633c --- /dev/null +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs @@ -0,0 +1,20 @@ +#![deny(unused_assignments, unused_variables)] +struct Object; + +fn change_object(mut object: &Object) { + let object2 = Object; + object = object2; //~ ERROR mismatched types +} + +fn change_object2(mut object: &Object) { //~ ERROR variable `object` is assigned to, but never used + let object2 = Object; + object = &object2; + //~^ ERROR `object2` does not live long enough + //~| ERROR value assigned to `object` is never read +} + +fn main() { + let object = Object; + change_object(&object); + change_object2(&object); +} diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr new file mode 100644 index 0000000000000..5ac5a8fa40db6 --- /dev/null +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr @@ -0,0 +1,60 @@ +error[E0308]: mismatched types + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:6:14 + | +LL | fn change_object(mut object: &Object) { + | ------- expected due to this parameter type +LL | let object2 = Object; +LL | object = object2; + | ^^^^^^^ expected `&Object`, found `Object` + | +help: consider borrowing here + | +LL | object = &object2; + | + + +error: value assigned to `object` is never read + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:5 + | +LL | object = &object2; + | ^^^^^^ + | + = help: maybe it is overwritten before being read? +note: the lint level is defined here + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:9 + | +LL | #![deny(unused_assignments, unused_variables)] + | ^^^^^^^^^^^^^^^^^^ + +error: variable `object` is assigned to, but never used + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:9:23 + | +LL | fn change_object2(mut object: &Object) { + | ^^^^^^ + | + = note: consider using `_object` instead +note: the lint level is defined here + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:29 + | +LL | #![deny(unused_assignments, unused_variables)] + | ^^^^^^^^^^^^^^^^ + +error[E0597]: `object2` does not live long enough + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:14 + | +LL | fn change_object2(mut object: &Object) { + | - let's call the lifetime of this reference `'1` +LL | let object2 = Object; + | ------- binding `object2` declared here +LL | object = &object2; + | ---------^^^^^^^^ + | | | + | | borrowed value does not live long enough + | assignment requires that `object2` is borrowed for `'1` +... +LL | } + | - `object2` dropped here while still borrowed + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0308, E0597. +For more information about an error, try `rustc --explain E0308`. From c2ae386c851b9f85439375dc788a506e2482ca60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 31 Dec 2024 17:13:11 +0000 Subject: [PATCH 03/14] On E0308, detect `mut arg: &Ty` meant to be `arg: &mut Ty` ``` error[E0308]: mismatched types --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:6:14 | LL | fn change_object(mut object: &Object) { | ------- expected due to this parameter type LL | let object2 = Object; LL | object = object2; | ^^^^^^^ expected `&Object`, found `Object` | help: you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding | LL ~ fn change_object(object: &mut Object) { LL | let object2 = Object; LL ~ *object = object2; | ``` This might be the first thing someone tries to write to mutate the value *behind* an argument. We avoid suggesting `object = &object2;`, as that is less likely to be what was intended. --- compiler/rustc_hir_typeck/src/demand.rs | 105 ++++++++++++++++++ ...-type-meant-to-be-arg-of-mut-borrow.stderr | 8 +- 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 56f7a2c1150ae..557e532ca8c6b 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -85,6 +85,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.annotate_expected_due_to_let_ty(err, expr, error); self.annotate_loop_expected_due_to_inference(err, expr, error); + if self.annotate_mut_binding_to_immutable_binding(err, expr, error) { + return; + } // FIXME(#73154): For now, we do leak check when coercing function // pointers in typeck, instead of only during borrowck. This can lead @@ -795,6 +798,108 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + /// Detect the following case + /// + /// ```text + /// fn change_object(mut a: &Ty) { + /// let a = Ty::new(); + /// b = a; + /// } + /// ``` + /// + /// where the user likely meant to modify the value behind there reference, use `a` as an out + /// parameter, instead of mutating the local binding. When encountering this we suggest: + /// + /// ```text + /// fn change_object(a: &'_ mut Ty) { + /// let a = Ty::new(); + /// *b = a; + /// } + /// ``` + fn annotate_mut_binding_to_immutable_binding( + &self, + err: &mut Diag<'_>, + expr: &hir::Expr<'_>, + error: Option>, + ) -> bool { + let Some(TypeError::Sorts(ExpectedFound { expected, found })) = error else { return false }; + let ty::Ref(_, inner, hir::Mutability::Not) = expected.kind() else { return false }; + if !self.can_eq(self.param_env, *inner, found) { + // The difference between the expected and found values isn't one level of borrowing. + return false; + } + // We have an `ident = expr;` assignment. + let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Assign(lhs, rhs, _), .. }) = + self.tcx.parent_hir_node(expr.hir_id) + else { + return false; + }; + if rhs.hir_id != expr.hir_id || expected.is_closure() { + return false; + } + // We are assigning to some binding. + let hir::ExprKind::Path(hir::QPath::Resolved( + None, + hir::Path { res: hir::def::Res::Local(hir_id), .. }, + )) = lhs.kind + else { + return false; + }; + let hir::Node::Pat(pat) = self.tcx.hir_node(*hir_id) else { return false }; + // The pattern we have is an fn argument. + let hir::Node::Param(hir::Param { ty_span, .. }) = self.tcx.parent_hir_node(pat.hir_id) + else { + return false; + }; + let item = self.tcx.hir().get_parent_item(pat.hir_id); + let item = self.tcx.hir_owner_node(item); + let Some(fn_decl) = item.fn_decl() else { return false }; + + // We have a mutable binding in the argument. + let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind else { + return false; + }; + + // Look for the type corresponding to the argument pattern we have in the argument list. + let Some(ty_sugg) = fn_decl + .inputs + .iter() + .filter_map(|ty| { + if ty.span == *ty_span + && let hir::TyKind::Ref(lt, mut_ty) = ty.kind + { + // `&'name Ty` -> `&'name mut Ty` or `&Ty` -> `&mut Ty` + Some(( + mut_ty.ty.span.shrink_to_lo(), + format!( + "{}mut ", + if lt.ident.span.lo() == lt.ident.span.hi() { "" } else { " " } + ), + )) + } else { + None + } + }) + .next() + else { + return false; + }; + let sugg = vec![ + ty_sugg, + (pat.span.until(ident.span), String::new()), + (lhs.span.shrink_to_lo(), "*".to_string()), + ]; + // We suggest changing the argument from `mut ident: &Ty` to `ident: &'_ mut Ty` and the + // assignment from `ident = val;` to `*ident = val;`. + err.multipart_suggestion_verbose( + "you might have meant to mutate the pointed at value being passed in, instead of \ + changing the reference in the local binding", + sugg, + Applicability::MaybeIncorrect, + ); + return true; + } + fn annotate_alternative_method_deref( &self, err: &mut Diag<'_>, diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr index 5ac5a8fa40db6..f4e1b599c5ab9 100644 --- a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr @@ -7,10 +7,12 @@ LL | let object2 = Object; LL | object = object2; | ^^^^^^^ expected `&Object`, found `Object` | -help: consider borrowing here +help: you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding + | +LL ~ fn change_object(object: &mut Object) { +LL | let object2 = Object; +LL ~ *object = object2; | -LL | object = &object2; - | + error: value assigned to `object` is never read --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:5 From ec98df4bb6839d017911444499557ecb73e39a00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 31 Dec 2024 17:33:45 +0000 Subject: [PATCH 04/14] On unused assign lint, detect `mut arg: &Ty` meant to be `arg: &mut Ty` ``` error: value assigned to `object` is never read --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:5 | LL | object = &object2; | ^^^^^^ | note: the lint level is defined here --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:9 | LL | #![deny(unused_assignments, unused_variables)] | ^^^^^^^^^^^^^^^^^^ help: you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding | LL ~ fn change_object2(object: &mut Object) { LL | let object2 = Object; LL ~ *object = object2; | ``` This might be the first thing someone tries to write to mutate the value *behind* an argument, trying to avoid an E0308. --- compiler/rustc_passes/messages.ftl | 3 + compiler/rustc_passes/src/errors.rs | 19 ++++- compiler/rustc_passes/src/liveness.rs | 81 ++++++++++++++++++- ...-type-meant-to-be-arg-of-mut-borrow.stderr | 7 +- 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 9df396cbf7a8c..3ed600a717f53 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -799,6 +799,9 @@ passes_unused_assign = value assigned to `{$name}` is never read passes_unused_assign_passed = value passed to `{$name}` is never read .help = maybe it is overwritten before being read? +passes_unused_assign_suggestion = + you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding + passes_unused_capture_maybe_capture_ref = value captured by `{$name}` is never read .help = did you mean to capture by reference instead? diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 13da021c6143a..c3043ac60aa69 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1787,9 +1787,26 @@ pub(crate) struct IneffectiveUnstableImpl; #[derive(LintDiagnostic)] #[diag(passes_unused_assign)] -#[help] pub(crate) struct UnusedAssign { pub name: String, + #[subdiagnostic] + pub suggestion: Option, + #[help] + pub help: bool, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(passes_unused_assign_suggestion, applicability = "maybe-incorrect")] +pub(crate) struct UnusedAssignSuggestion { + pub pre: &'static str, + #[suggestion_part(code = "{pre}mut ")] + pub ty_span: Span, + #[suggestion_part(code = "")] + pub ty_ref_span: Span, + #[suggestion_part(code = "*")] + pub ident_span: Span, + #[suggestion_part(code = "")] + pub expr_ref_span: Span, } #[derive(LintDiagnostic)] diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index b85a987c64119..426899a4d5c6d 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -1360,7 +1360,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> { fn visit_local(&mut self, local: &'tcx hir::LetStmt<'tcx>) { self.check_unused_vars_in_pat(local.pat, None, None, |spans, hir_id, ln, var| { if local.init.is_some() { - self.warn_about_dead_assign(spans, hir_id, ln, var); + self.warn_about_dead_assign(spans, hir_id, ln, var, None); } }); @@ -1460,7 +1460,8 @@ impl<'tcx> Liveness<'_, 'tcx> { // as being used. let ln = self.live_node(expr.hir_id, expr.span); let var = self.variable(var_hid, expr.span); - self.warn_about_dead_assign(vec![expr.span], expr.hir_id, ln, var); + let sugg = self.annotate_mut_binding_to_immutable_binding(var_hid, expr); + self.warn_about_dead_assign(vec![expr.span], expr.hir_id, ln, var, sugg); } } _ => { @@ -1585,6 +1586,70 @@ impl<'tcx> Liveness<'_, 'tcx> { } } + /// Detect the following case + /// + /// ```text + /// fn change_object(mut a: &Ty) { + /// let a = Ty::new(); + /// b = &a; + /// } + /// ``` + /// + /// where the user likely meant to modify the value behind there reference, use `a` as an out + /// parameter, instead of mutating the local binding. When encountering this we suggest: + /// + /// ```text + /// fn change_object(a: &'_ mut Ty) { + /// let a = Ty::new(); + /// *b = a; + /// } + /// ``` + fn annotate_mut_binding_to_immutable_binding( + &self, + var_hid: HirId, + expr: &'tcx Expr<'tcx>, + ) -> Option { + if let hir::Node::Expr(parent) = self.ir.tcx.parent_hir_node(expr.hir_id) + && let hir::ExprKind::Assign(_, rhs, _) = parent.kind + && let hir::ExprKind::AddrOf(borrow_kind, _mut, inner) = rhs.kind + && let hir::BorrowKind::Ref = borrow_kind + && let hir::Node::Pat(pat) = self.ir.tcx.hir_node(var_hid) + && let hir::Node::Param(hir::Param { ty_span, .. }) = + self.ir.tcx.parent_hir_node(pat.hir_id) + && let item_id = self.ir.tcx.hir().get_parent_item(pat.hir_id) + && let item = self.ir.tcx.hir_owner_node(item_id) + && let Some(fn_decl) = item.fn_decl() + && let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind + && let Some((ty_span, pre)) = fn_decl + .inputs + .iter() + .filter_map(|ty| { + if ty.span == *ty_span + && let hir::TyKind::Ref(lt, mut_ty) = ty.kind + { + // `&'name Ty` -> `&'name mut Ty` or `&Ty` -> `&mut Ty` + Some(( + mut_ty.ty.span.shrink_to_lo(), + if lt.ident.span.lo() == lt.ident.span.hi() { "" } else { " " }, + )) + } else { + None + } + }) + .next() + { + Some(errors::UnusedAssignSuggestion { + ty_span, + pre, + ty_ref_span: pat.span.until(ident.span), + ident_span: expr.span.shrink_to_lo(), + expr_ref_span: rhs.span.until(inner.span), + }) + } else { + None + } + } + #[instrument(skip(self), level = "INFO")] fn report_unused( &self, @@ -1738,15 +1803,23 @@ impl<'tcx> Liveness<'_, 'tcx> { suggs } - fn warn_about_dead_assign(&self, spans: Vec, hir_id: HirId, ln: LiveNode, var: Variable) { + fn warn_about_dead_assign( + &self, + spans: Vec, + hir_id: HirId, + ln: LiveNode, + var: Variable, + suggestion: Option, + ) { if !self.live_on_exit(ln, var) && let Some(name) = self.should_warn(var) { + let help = suggestion.is_none(); self.ir.tcx.emit_node_span_lint( lint::builtin::UNUSED_ASSIGNMENTS, hir_id, spans, - errors::UnusedAssign { name }, + errors::UnusedAssign { name, suggestion, help }, ); } } diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr index f4e1b599c5ab9..edb1525c0d597 100644 --- a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr @@ -20,12 +20,17 @@ error: value assigned to `object` is never read LL | object = &object2; | ^^^^^^ | - = help: maybe it is overwritten before being read? note: the lint level is defined here --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:9 | LL | #![deny(unused_assignments, unused_variables)] | ^^^^^^^^^^^^^^^^^^ +help: you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding + | +LL ~ fn change_object2(object: &mut Object) { +LL | let object2 = Object; +LL ~ *object = object2; + | error: variable `object` is assigned to, but never used --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:9:23 From 4438b3211f32f85145a9d30fcd107206899d722c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 11 Jan 2025 01:58:32 +0000 Subject: [PATCH 05/14] review comments and make test `run-rustfix` --- compiler/rustc_hir_typeck/src/demand.rs | 138 ++++++++---------- ...d-type-meant-to-be-arg-of-mut-borrow.fixed | 22 +++ ...owed-type-meant-to-be-arg-of-mut-borrow.rs | 10 +- ...-type-meant-to-be-arg-of-mut-borrow.stderr | 14 +- 4 files changed, 100 insertions(+), 84 deletions(-) create mode 100644 tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.fixed diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 557e532ca8c6b..367e7c6de9532 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -822,82 +822,72 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr: &hir::Expr<'_>, error: Option>, ) -> bool { - let Some(TypeError::Sorts(ExpectedFound { expected, found })) = error else { return false }; - let ty::Ref(_, inner, hir::Mutability::Not) = expected.kind() else { return false }; - if !self.can_eq(self.param_env, *inner, found) { - // The difference between the expected and found values isn't one level of borrowing. - return false; - } - // We have an `ident = expr;` assignment. - let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Assign(lhs, rhs, _), .. }) = - self.tcx.parent_hir_node(expr.hir_id) - else { - return false; - }; - if rhs.hir_id != expr.hir_id || expected.is_closure() { - return false; - } - // We are assigning to some binding. - let hir::ExprKind::Path(hir::QPath::Resolved( - None, - hir::Path { res: hir::def::Res::Local(hir_id), .. }, - )) = lhs.kind - else { - return false; - }; - let hir::Node::Pat(pat) = self.tcx.hir_node(*hir_id) else { return false }; - // The pattern we have is an fn argument. - let hir::Node::Param(hir::Param { ty_span, .. }) = self.tcx.parent_hir_node(pat.hir_id) - else { - return false; - }; - let item = self.tcx.hir().get_parent_item(pat.hir_id); - let item = self.tcx.hir_owner_node(item); - let Some(fn_decl) = item.fn_decl() else { return false }; + if let Some(TypeError::Sorts(ExpectedFound { expected, found })) = error + && let ty::Ref(_, inner, hir::Mutability::Not) = expected.kind() - // We have a mutable binding in the argument. - let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind else { - return false; - }; + // The difference between the expected and found values is one level of borrowing. + && self.can_eq(self.param_env, *inner, found) - // Look for the type corresponding to the argument pattern we have in the argument list. - let Some(ty_sugg) = fn_decl - .inputs - .iter() - .filter_map(|ty| { - if ty.span == *ty_span - && let hir::TyKind::Ref(lt, mut_ty) = ty.kind - { - // `&'name Ty` -> `&'name mut Ty` or `&Ty` -> `&mut Ty` - Some(( - mut_ty.ty.span.shrink_to_lo(), - format!( - "{}mut ", - if lt.ident.span.lo() == lt.ident.span.hi() { "" } else { " " } - ), - )) - } else { - None - } - }) - .next() - else { - return false; - }; - let sugg = vec![ - ty_sugg, - (pat.span.until(ident.span), String::new()), - (lhs.span.shrink_to_lo(), "*".to_string()), - ]; - // We suggest changing the argument from `mut ident: &Ty` to `ident: &'_ mut Ty` and the - // assignment from `ident = val;` to `*ident = val;`. - err.multipart_suggestion_verbose( - "you might have meant to mutate the pointed at value being passed in, instead of \ - changing the reference in the local binding", - sugg, - Applicability::MaybeIncorrect, - ); - return true; + // We have an `ident = expr;` assignment. + && let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Assign(lhs, rhs, _), .. }) = + self.tcx.parent_hir_node(expr.hir_id) + && rhs.hir_id == expr.hir_id + + // We are assigning to some binding. + && let hir::ExprKind::Path(hir::QPath::Resolved( + None, + hir::Path { res: hir::def::Res::Local(hir_id), .. }, + )) = lhs.kind + && let hir::Node::Pat(pat) = self.tcx.hir_node(*hir_id) + + // The pattern we have is an fn argument. + && let hir::Node::Param(hir::Param { ty_span, .. }) = + self.tcx.parent_hir_node(pat.hir_id) + && let item = self.tcx.hir().get_parent_item(pat.hir_id) + && let item = self.tcx.hir_owner_node(item) + && let Some(fn_decl) = item.fn_decl() + + // We have a mutable binding in the argument. + && let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind + + // Look for the type corresponding to the argument pattern we have in the argument list. + && let Some(ty_sugg) = fn_decl + .inputs + .iter() + .filter_map(|ty| { + if ty.span == *ty_span + && let hir::TyKind::Ref(lt, x) = ty.kind + { + // `&'name Ty` -> `&'name mut Ty` or `&Ty` -> `&mut Ty` + Some(( + x.ty.span.shrink_to_lo(), + format!( + "{}mut ", + if lt.ident.span.lo() == lt.ident.span.hi() { "" } else { " " } + ), + )) + } else { + None + } + }) + .next() + { + let sugg = vec![ + ty_sugg, + (pat.span.until(ident.span), String::new()), + (lhs.span.shrink_to_lo(), "*".to_string()), + ]; + // We suggest changing the argument from `mut ident: &Ty` to `ident: &'_ mut Ty` and the + // assignment from `ident = val;` to `*ident = val;`. + err.multipart_suggestion_verbose( + "you might have meant to mutate the pointed at value being passed in, instead of \ + changing the reference in the local binding", + sugg, + Applicability::MaybeIncorrect, + ); + return true; + } + false } fn annotate_alternative_method_deref( diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.fixed b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.fixed new file mode 100644 index 0000000000000..914ca1f3a065b --- /dev/null +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.fixed @@ -0,0 +1,22 @@ +//@ run-rustfix +#![deny(unused_assignments, unused_variables)] +struct Object; + +fn change_object(object: &mut Object) { //~ HELP you might have meant to mutate + let object2 = Object; + *object = object2; //~ ERROR mismatched types +} + +fn change_object2(object: &mut Object) { //~ ERROR variable `object` is assigned to, but never used + //~^ HELP you might have meant to mutate + let object2 = Object; + *object = object2; + //~^ ERROR `object2` does not live long enough + //~| ERROR value assigned to `object` is never read +} + +fn main() { + let mut object = Object; + change_object(&mut object); + change_object2(&mut object); +} diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs index eff4745e4633c..331359a98d147 100644 --- a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs @@ -1,12 +1,14 @@ +//@ run-rustfix #![deny(unused_assignments, unused_variables)] struct Object; -fn change_object(mut object: &Object) { +fn change_object(mut object: &Object) { //~ HELP you might have meant to mutate let object2 = Object; object = object2; //~ ERROR mismatched types } fn change_object2(mut object: &Object) { //~ ERROR variable `object` is assigned to, but never used + //~^ HELP you might have meant to mutate let object2 = Object; object = &object2; //~^ ERROR `object2` does not live long enough @@ -14,7 +16,7 @@ fn change_object2(mut object: &Object) { //~ ERROR variable `object` is assigned } fn main() { - let object = Object; - change_object(&object); - change_object2(&object); + let mut object = Object; + change_object(&mut object); + change_object2(&mut object); } diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr index edb1525c0d597..e7e4003936a11 100644 --- a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:6:14 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:7:14 | LL | fn change_object(mut object: &Object) { | ------- expected due to this parameter type @@ -15,41 +15,43 @@ LL ~ *object = object2; | error: value assigned to `object` is never read - --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:5 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:13:5 | LL | object = &object2; | ^^^^^^ | note: the lint level is defined here - --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:9 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:2:9 | LL | #![deny(unused_assignments, unused_variables)] | ^^^^^^^^^^^^^^^^^^ help: you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding | LL ~ fn change_object2(object: &mut Object) { +LL | LL | let object2 = Object; LL ~ *object = object2; | error: variable `object` is assigned to, but never used - --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:9:23 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:10:23 | LL | fn change_object2(mut object: &Object) { | ^^^^^^ | = note: consider using `_object` instead note: the lint level is defined here - --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:29 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:2:29 | LL | #![deny(unused_assignments, unused_variables)] | ^^^^^^^^^^^^^^^^ error[E0597]: `object2` does not live long enough - --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:14 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:13:14 | LL | fn change_object2(mut object: &Object) { | - let's call the lifetime of this reference `'1` +LL | LL | let object2 = Object; | ------- binding `object2` declared here LL | object = &object2; From 6f833aa057f7db01dcdc919c66bbd0a179c58ad4 Mon Sep 17 00:00:00 2001 From: Ross Sullivan Date: Sun, 12 Jan 2025 12:19:42 +0900 Subject: [PATCH 06/14] re-added regression test for #122638 --- ...malizing_with_unconstrained_impl_params.rs | 18 ++++++ ...zing_with_unconstrained_impl_params.stderr | 60 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 tests/ui/const-generics/normalizing_with_unconstrained_impl_params.rs create mode 100644 tests/ui/const-generics/normalizing_with_unconstrained_impl_params.stderr diff --git a/tests/ui/const-generics/normalizing_with_unconstrained_impl_params.rs b/tests/ui/const-generics/normalizing_with_unconstrained_impl_params.rs new file mode 100644 index 0000000000000..ba37087135fe6 --- /dev/null +++ b/tests/ui/const-generics/normalizing_with_unconstrained_impl_params.rs @@ -0,0 +1,18 @@ +//! Regression test for . +//@ check-fail +#![feature(min_specialization)] +impl<'a, T: std::fmt::Debug, const N: usize> Iterator for ConstChunksExact<'a, T, { N }> { + //~^ ERROR not all trait items implemented, missing: `Item` [E0046] + fn next(&mut self) -> Option {} + //~^ ERROR mismatched types [E0308] +} +struct ConstChunksExact<'a, T: '_, const assert: usize> {} +//~^ ERROR `'_` cannot be used here [E0637] +//~| ERROR lifetime parameter `'a` is never used [E0392] +//~| ERROR type parameter `T` is never used [E0392] +impl<'a, T: std::fmt::Debug, const N: usize> Iterator for ConstChunksExact<'a, T, {}> { + //~^ ERROR mismatched types [E0308] + //~| ERROR the const parameter `N` is not constrained by the impl trait, self type, or predicates [E0207] + type Item = &'a [T; N]; } + +fn main() {} diff --git a/tests/ui/const-generics/normalizing_with_unconstrained_impl_params.stderr b/tests/ui/const-generics/normalizing_with_unconstrained_impl_params.stderr new file mode 100644 index 0000000000000..1ee6864759471 --- /dev/null +++ b/tests/ui/const-generics/normalizing_with_unconstrained_impl_params.stderr @@ -0,0 +1,60 @@ +error[E0637]: `'_` cannot be used here + --> $DIR/normalizing_with_unconstrained_impl_params.rs:9:32 + | +LL | struct ConstChunksExact<'a, T: '_, const assert: usize> {} + | ^^ `'_` is a reserved lifetime name + +error[E0308]: mismatched types + --> $DIR/normalizing_with_unconstrained_impl_params.rs:13:83 + | +LL | impl<'a, T: std::fmt::Debug, const N: usize> Iterator for ConstChunksExact<'a, T, {}> { + | ^^ expected `usize`, found `()` + +error[E0046]: not all trait items implemented, missing: `Item` + --> $DIR/normalizing_with_unconstrained_impl_params.rs:4:1 + | +LL | impl<'a, T: std::fmt::Debug, const N: usize> Iterator for ConstChunksExact<'a, T, { N }> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `Item` in implementation + | + = help: implement the missing item: `type Item = /* Type */;` + +error[E0392]: lifetime parameter `'a` is never used + --> $DIR/normalizing_with_unconstrained_impl_params.rs:9:25 + | +LL | struct ConstChunksExact<'a, T: '_, const assert: usize> {} + | ^^ unused lifetime parameter + | + = help: consider removing `'a`, referring to it in a field, or using a marker such as `PhantomData` + +error[E0392]: type parameter `T` is never used + --> $DIR/normalizing_with_unconstrained_impl_params.rs:9:29 + | +LL | struct ConstChunksExact<'a, T: '_, const assert: usize> {} + | ^ unused type parameter + | + = help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData` + +error[E0207]: the const parameter `N` is not constrained by the impl trait, self type, or predicates + --> $DIR/normalizing_with_unconstrained_impl_params.rs:13:30 + | +LL | impl<'a, T: std::fmt::Debug, const N: usize> Iterator for ConstChunksExact<'a, T, {}> { + | ^^^^^^^^^^^^^^ unconstrained const parameter + | + = note: expressions using a const parameter must map each value to a distinct output value + = note: proving the result of expressions other than the parameter are unique is not supported + +error[E0308]: mismatched types + --> $DIR/normalizing_with_unconstrained_impl_params.rs:6:27 + | +LL | fn next(&mut self) -> Option {} + | ---- ^^^^^^^^^^^^^^^^^^ expected `Option<_>`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + | + = note: expected enum `Option<_>` + found unit type `()` + +error: aborting due to 7 previous errors + +Some errors have detailed explanations: E0046, E0207, E0308, E0392, E0637. +For more information about an error, try `rustc --explain E0046`. From 87f03a4238d5416eedad758dc64e09c3f7cc5df4 Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 13 Jan 2025 14:14:57 +0100 Subject: [PATCH 07/14] rm unnecessary `OpaqueTypeDecl` wrapper --- .../src/type_check/opaque_types.rs | 4 ++-- .../rustc_hir_analysis/src/check/check.rs | 6 +++--- compiler/rustc_hir_typeck/src/writeback.rs | 6 +++--- .../src/infer/canonical/query_response.rs | 4 ++-- compiler/rustc_infer/src/infer/mod.rs | 2 +- .../rustc_infer/src/infer/opaque_types/mod.rs | 13 +----------- .../src/infer/opaque_types/table.rs | 21 +++++++++++-------- 7 files changed, 24 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/opaque_types.rs b/compiler/rustc_borrowck/src/type_check/opaque_types.rs index edf3b1ae092da..ad4e006c21ae8 100644 --- a/compiler/rustc_borrowck/src/type_check/opaque_types.rs +++ b/compiler/rustc_borrowck/src/type_check/opaque_types.rs @@ -25,8 +25,8 @@ pub(super) fn take_opaques_and_register_member_constraints<'tcx>( let opaque_types = infcx .take_opaque_types() .into_iter() - .map(|(opaque_type_key, decl)| { - let hidden_type = infcx.resolve_vars_if_possible(decl.hidden_type); + .map(|(opaque_type_key, hidden_type)| { + let hidden_type = infcx.resolve_vars_if_possible(hidden_type); register_member_constraints( typeck, &mut member_constraints, diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 974a8648bc0e0..ec82644ea5ba4 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -436,9 +436,9 @@ fn check_opaque_meets_bounds<'tcx>( } else { // Check that any hidden types found during wf checking match the hidden types that `type_of` sees. for (mut key, mut ty) in infcx.take_opaque_types() { - ty.hidden_type.ty = infcx.resolve_vars_if_possible(ty.hidden_type.ty); + ty.ty = infcx.resolve_vars_if_possible(ty.ty); key = infcx.resolve_vars_if_possible(key); - sanity_check_found_hidden_type(tcx, key, ty.hidden_type)?; + sanity_check_found_hidden_type(tcx, key, ty)?; } Ok(()) } @@ -1873,7 +1873,7 @@ pub(super) fn check_coroutine_obligations( // Check that any hidden types found when checking these stalled coroutine obligations // are valid. for (key, ty) in infcx.take_opaque_types() { - let hidden_type = infcx.resolve_vars_if_possible(ty.hidden_type); + let hidden_type = infcx.resolve_vars_if_possible(ty); let key = infcx.resolve_vars_if_possible(key); sanity_check_found_hidden_type(tcx, key, hidden_type)?; } diff --git a/compiler/rustc_hir_typeck/src/writeback.rs b/compiler/rustc_hir_typeck/src/writeback.rs index 7c5838db58664..683cacdff7d78 100644 --- a/compiler/rustc_hir_typeck/src/writeback.rs +++ b/compiler/rustc_hir_typeck/src/writeback.rs @@ -562,9 +562,9 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { // types or by using this function at the end of writeback and running it as a // fixpoint. let opaque_types = self.fcx.infcx.clone_opaque_types(); - for (opaque_type_key, decl) in opaque_types { - let hidden_type = self.resolve(decl.hidden_type, &decl.hidden_type.span); - let opaque_type_key = self.resolve(opaque_type_key, &decl.hidden_type.span); + for (opaque_type_key, hidden_type) in opaque_types { + let hidden_type = self.resolve(hidden_type, &hidden_type.span); + let opaque_type_key = self.resolve(opaque_type_key, &hidden_type.span); if !self.fcx.next_trait_solver() { if let ty::Alias(ty::Opaque, alias_ty) = hidden_type.ty.kind() diff --git a/compiler/rustc_infer/src/infer/canonical/query_response.rs b/compiler/rustc_infer/src/infer/canonical/query_response.rs index d5aab4781de86..23f63af778d07 100644 --- a/compiler/rustc_infer/src/infer/canonical/query_response.rs +++ b/compiler/rustc_infer/src/infer/canonical/query_response.rs @@ -155,12 +155,12 @@ impl<'tcx> InferCtxt<'tcx> { .opaque_type_storage .opaque_types .iter() - .map(|(k, v)| (*k, v.hidden_type.ty)) + .map(|(k, v)| (*k, v.ty)) .collect() } fn take_opaque_types_for_query_response(&self) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> { - self.take_opaque_types().into_iter().map(|(k, v)| (k, v.hidden_type.ty)).collect() + self.take_opaque_types().into_iter().map(|(k, v)| (k, v.ty)).collect() } /// Given the (canonicalized) result to a canonical query, diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 1f7180fb80a0c..283ebdfa236be 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -234,7 +234,7 @@ impl<'tcx> InferCtxtInner<'tcx> { pub fn iter_opaque_types( &self, ) -> impl Iterator, ty::OpaqueHiddenType<'tcx>)> + '_ { - self.opaque_type_storage.opaque_types.iter().map(|(&k, v)| (k, v.hidden_type)) + self.opaque_type_storage.opaque_types.iter().map(|(&k, &v)| (k, v)) } } diff --git a/compiler/rustc_infer/src/infer/opaque_types/mod.rs b/compiler/rustc_infer/src/infer/opaque_types/mod.rs index 137d438a47955..f6ef3f40e622d 100644 --- a/compiler/rustc_infer/src/infer/opaque_types/mod.rs +++ b/compiler/rustc_infer/src/infer/opaque_types/mod.rs @@ -19,20 +19,9 @@ use crate::traits::{self, Obligation, PredicateObligations}; mod table; -pub(crate) type OpaqueTypeMap<'tcx> = FxIndexMap, OpaqueTypeDecl<'tcx>>; +pub(crate) type OpaqueTypeMap<'tcx> = FxIndexMap, OpaqueHiddenType<'tcx>>; pub(crate) use table::{OpaqueTypeStorage, OpaqueTypeTable}; -/// Information about the opaque types whose values we -/// are inferring in this function (these are the `impl Trait` that -/// appear in the return type). -#[derive(Clone, Debug)] -pub struct OpaqueTypeDecl<'tcx> { - /// The hidden types that have been inferred for this opaque type. - /// There can be multiple, but they are all `lub`ed together at the end - /// to obtain the canonical hidden type. - pub hidden_type: OpaqueHiddenType<'tcx>, -} - impl<'tcx> InferCtxt<'tcx> { /// This is a backwards compatibility hack to prevent breaking changes from /// lazy TAIT around RPIT handling. diff --git a/compiler/rustc_infer/src/infer/opaque_types/table.rs b/compiler/rustc_infer/src/infer/opaque_types/table.rs index 047d8edad3de7..ba6cc0d783dd3 100644 --- a/compiler/rustc_infer/src/infer/opaque_types/table.rs +++ b/compiler/rustc_infer/src/infer/opaque_types/table.rs @@ -3,7 +3,7 @@ use rustc_middle::bug; use rustc_middle::ty::{self, OpaqueHiddenType, OpaqueTypeKey, Ty}; use tracing::instrument; -use super::{OpaqueTypeDecl, OpaqueTypeMap}; +use super::OpaqueTypeMap; use crate::infer::snapshot::undo_log::{InferCtxtUndoLogs, UndoLog}; #[derive(Default, Debug, Clone)] @@ -11,15 +11,19 @@ pub(crate) struct OpaqueTypeStorage<'tcx> { /// Opaque types found in explicit return types and their /// associated fresh inference variable. Writeback resolves these /// variables to get the concrete type, which can be used to - /// 'de-opaque' OpaqueTypeDecl, after typeck is done with all functions. + /// 'de-opaque' OpaqueHiddenType, after typeck is done with all functions. pub opaque_types: OpaqueTypeMap<'tcx>, } impl<'tcx> OpaqueTypeStorage<'tcx> { #[instrument(level = "debug")] - pub(crate) fn remove(&mut self, key: OpaqueTypeKey<'tcx>, idx: Option>) { - if let Some(idx) = idx { - self.opaque_types.get_mut(&key).unwrap().hidden_type = idx; + pub(crate) fn remove( + &mut self, + key: OpaqueTypeKey<'tcx>, + prev: Option>, + ) { + if let Some(prev) = prev { + *self.opaque_types.get_mut(&key).unwrap() = prev; } else { // FIXME(#120456) - is `swap_remove` correct? match self.opaque_types.swap_remove(&key) { @@ -59,13 +63,12 @@ impl<'a, 'tcx> OpaqueTypeTable<'a, 'tcx> { key: OpaqueTypeKey<'tcx>, hidden_type: OpaqueHiddenType<'tcx>, ) -> Option> { - if let Some(decl) = self.storage.opaque_types.get_mut(&key) { - let prev = std::mem::replace(&mut decl.hidden_type, hidden_type); + if let Some(entry) = self.storage.opaque_types.get_mut(&key) { + let prev = std::mem::replace(entry, hidden_type); self.undo_log.push(UndoLog::OpaqueTypes(key, Some(prev))); return Some(prev.ty); } - let decl = OpaqueTypeDecl { hidden_type }; - self.storage.opaque_types.insert(key, decl); + self.storage.opaque_types.insert(key, hidden_type); self.undo_log.push(UndoLog::OpaqueTypes(key, None)); None } From 1b068a0dea794373301f4ddbfb35a378c6805276 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 13 Jan 2025 15:52:45 +0000 Subject: [PATCH 08/14] Make sure to mark IMPL_TRAIT_REDUNDANT_CAPTURES as Allow in edition 2024 --- .../rustc_lint/src/impl_trait_overcaptures.rs | 2 +- .../ui/impl-trait/precise-capturing/redundant.rs | 10 +++++----- .../precise-capturing/redundant.stderr | 16 ++++++++++------ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 7f603f6a65531..44f8653552711 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -99,7 +99,7 @@ declare_lint! { /// To fix this, remove the `use<'a>`, since the lifetime is already captured /// since it is in scope. pub IMPL_TRAIT_REDUNDANT_CAPTURES, - Warn, + Allow, "redundant precise-capturing `use<...>` syntax on an `impl Trait`", } diff --git a/tests/ui/impl-trait/precise-capturing/redundant.rs b/tests/ui/impl-trait/precise-capturing/redundant.rs index 075d7c70ac653..32dc09273175d 100644 --- a/tests/ui/impl-trait/precise-capturing/redundant.rs +++ b/tests/ui/impl-trait/precise-capturing/redundant.rs @@ -1,24 +1,24 @@ //@ edition: 2024 -//@ check-pass #![feature(precise_capturing_in_traits)] +#![deny(impl_trait_redundant_captures)] fn hello<'a>() -> impl Sized + use<'a> {} -//~^ WARN all possible in-scope parameters are already captured +//~^ ERROR all possible in-scope parameters are already captured struct Inherent; impl Inherent { fn inherent(&self) -> impl Sized + use<'_> {} - //~^ WARN all possible in-scope parameters are already captured + //~^ ERROR all possible in-scope parameters are already captured } trait Test<'a> { fn in_trait() -> impl Sized + use<'a, Self>; - //~^ WARN all possible in-scope parameters are already captured + //~^ ERROR all possible in-scope parameters are already captured } impl<'a> Test<'a> for () { fn in_trait() -> impl Sized + use<'a> {} - //~^ WARN all possible in-scope parameters are already captured + //~^ ERROR all possible in-scope parameters are already captured } fn main() {} diff --git a/tests/ui/impl-trait/precise-capturing/redundant.stderr b/tests/ui/impl-trait/precise-capturing/redundant.stderr index 274d9d2375f7d..5c8b35c228585 100644 --- a/tests/ui/impl-trait/precise-capturing/redundant.stderr +++ b/tests/ui/impl-trait/precise-capturing/redundant.stderr @@ -1,4 +1,4 @@ -warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant +error: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant --> $DIR/redundant.rs:6:19 | LL | fn hello<'a>() -> impl Sized + use<'a> {} @@ -6,9 +6,13 @@ LL | fn hello<'a>() -> impl Sized + use<'a> {} | | | help: remove the `use<...>` syntax | - = note: `#[warn(impl_trait_redundant_captures)]` on by default +note: the lint level is defined here + --> $DIR/redundant.rs:4:9 + | +LL | #![deny(impl_trait_redundant_captures)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant +error: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant --> $DIR/redundant.rs:11:27 | LL | fn inherent(&self) -> impl Sized + use<'_> {} @@ -16,7 +20,7 @@ LL | fn inherent(&self) -> impl Sized + use<'_> {} | | | help: remove the `use<...>` syntax -warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant +error: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant --> $DIR/redundant.rs:16:22 | LL | fn in_trait() -> impl Sized + use<'a, Self>; @@ -24,7 +28,7 @@ LL | fn in_trait() -> impl Sized + use<'a, Self>; | | | help: remove the `use<...>` syntax -warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant +error: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant --> $DIR/redundant.rs:20:22 | LL | fn in_trait() -> impl Sized + use<'a> {} @@ -32,5 +36,5 @@ LL | fn in_trait() -> impl Sized + use<'a> {} | | | help: remove the `use<...>` syntax -warning: 4 warnings emitted +error: aborting due to 4 previous errors From b3ba4de8b29941f9a6826dd444aea38aad7cb726 Mon Sep 17 00:00:00 2001 From: rustbot <47979223+rustbot@users.noreply.github.com> Date: Mon, 13 Jan 2025 12:00:40 -0500 Subject: [PATCH 09/14] Update books --- src/doc/book | 2 +- src/doc/nomicon | 2 +- src/doc/reference | 2 +- src/doc/rust-by-example | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/doc/book b/src/doc/book index 04d06dfe54160..5a65e2af063ff 160000 --- a/src/doc/book +++ b/src/doc/book @@ -1 +1 @@ -Subproject commit 04d06dfe541607e6419f3d028c3f9b245f3be4d9 +Subproject commit 5a65e2af063ff701ae858f1f7536ee347b3cfe63 diff --git a/src/doc/nomicon b/src/doc/nomicon index 7ef05b9777c94..625b200e5b33a 160000 --- a/src/doc/nomicon +++ b/src/doc/nomicon @@ -1 +1 @@ -Subproject commit 7ef05b9777c94836bc92f50f23e6e00981521a89 +Subproject commit 625b200e5b33a5af35589db0bc454203a3d46d20 diff --git a/src/doc/reference b/src/doc/reference index acd6794e712d5..293af99100377 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit acd6794e712d5e2ef6f5c84fb95688d32a69b816 +Subproject commit 293af991003772bdccf2d6b980182d84dd055942 diff --git a/src/doc/rust-by-example b/src/doc/rust-by-example index 093397535b48a..054259ed1bf01 160000 --- a/src/doc/rust-by-example +++ b/src/doc/rust-by-example @@ -1 +1 @@ -Subproject commit 093397535b48ae13ec76bc526b7e6eb8c096a85c +Subproject commit 054259ed1bf01cdee4309ee764c7e103f6df3de5 From 75f3ebdc1ef8a32b48878a7961079c1bf8dd96e7 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 13 Jan 2025 10:10:40 -0800 Subject: [PATCH 10/14] Mark rustbook as an external tool It has been a bit of a pain trying to keep the lints in sync across the submodule repositories, so the just turns it off. --- src/bootstrap/src/core/build_steps/tool.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index d0058eeb43d3c..9ae03ac7fe075 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -334,7 +334,11 @@ macro_rules! bootstrap_tool { } bootstrap_tool!( - Rustbook, "src/tools/rustbook", "rustbook", submodules = SUBMODULES_FOR_RUSTBOOK; + // This is marked as an external tool because it includes dependencies + // from submodules. Trying to keep the lints in sync between all the repos + // is a bit of a pain. Unfortunately it means the rustbook source itself + // doesn't deny warnings, but it is a relatively small piece of code. + Rustbook, "src/tools/rustbook", "rustbook", is_external_tool = true, submodules = SUBMODULES_FOR_RUSTBOOK; UnstableBookGen, "src/tools/unstable-book-gen", "unstable-book-gen"; Tidy, "src/tools/tidy", "tidy"; Linkchecker, "src/tools/linkchecker", "linkchecker"; From 6e67ffa4f2af426396b8af2b6829e89212cce5ba Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Sun, 12 Jan 2025 10:01:45 +0530 Subject: [PATCH 11/14] uefi: helpers: Introduce OwnedDevicePath This PR is split off from #135368 to reduce noise. Rename DevicePath to OwnedDevicePath. This is to allow a non-owning version of DevicePath in the future to work with UEFI shell APIs which provide const pointers to device paths for UEFI shell fs mapping. Also implement Debug for OwnedDevicePath for some quality of life improvements. Signed-off-by: Ayush Singh --- library/std/src/sys/pal/uefi/helpers.rs | 21 +++++++++++++++------ library/std/src/sys/pal/uefi/process.rs | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/library/std/src/sys/pal/uefi/helpers.rs b/library/std/src/sys/pal/uefi/helpers.rs index 47d9add72b5f0..7504a0f7ad7f5 100644 --- a/library/std/src/sys/pal/uefi/helpers.rs +++ b/library/std/src/sys/pal/uefi/helpers.rs @@ -222,14 +222,14 @@ pub(crate) fn runtime_services() -> Option> NonNull::new(runtime_services) } -pub(crate) struct DevicePath(NonNull); +pub(crate) struct OwnedDevicePath(NonNull); -impl DevicePath { +impl OwnedDevicePath { pub(crate) fn from_text(p: &OsStr) -> io::Result { fn inner( p: &OsStr, protocol: NonNull, - ) -> io::Result { + ) -> io::Result { let path_vec = p.encode_wide().chain(Some(0)).collect::>(); if path_vec[..path_vec.len() - 1].contains(&0) { return Err(const_error!( @@ -242,7 +242,7 @@ impl DevicePath { unsafe { ((*protocol.as_ptr()).convert_text_to_device_path)(path_vec.as_ptr()) }; NonNull::new(path) - .map(DevicePath) + .map(OwnedDevicePath) .ok_or_else(|| const_error!(io::ErrorKind::InvalidFilename, "Invalid Device Path")) } @@ -275,12 +275,12 @@ impl DevicePath { )) } - pub(crate) fn as_ptr(&self) -> *mut r_efi::protocols::device_path::Protocol { + pub(crate) const fn as_ptr(&self) -> *mut r_efi::protocols::device_path::Protocol { self.0.as_ptr() } } -impl Drop for DevicePath { +impl Drop for OwnedDevicePath { fn drop(&mut self) { if let Some(bt) = boot_services() { let bt: NonNull = bt.cast(); @@ -291,6 +291,15 @@ impl Drop for DevicePath { } } +impl crate::fmt::Debug for OwnedDevicePath { + fn fmt(&self, f: &mut crate::fmt::Formatter<'_>) -> crate::fmt::Result { + match device_path_to_text(self.0) { + Ok(p) => p.fmt(f), + Err(_) => f.debug_struct("OwnedDevicePath").finish_non_exhaustive(), + } + } +} + pub(crate) struct OwnedProtocol { guid: r_efi::efi::Guid, handle: NonNull, diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index 95707ebb7f023..1a0754134dfb8 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -326,7 +326,7 @@ mod uefi_command_internal { impl Image { pub fn load_image(p: &OsStr) -> io::Result { - let path = helpers::DevicePath::from_text(p)?; + let path = helpers::OwnedDevicePath::from_text(p)?; let boot_services: NonNull = boot_services() .ok_or_else(|| const_error!(io::ErrorKind::NotFound, "Boot Services not found"))? .cast(); From a9f32406dad78dd0d33139f6a0bc8884d62ad594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Mon, 13 Jan 2025 16:58:09 +0800 Subject: [PATCH 12/14] bootstrap: fix outdated feature name in comment --- src/bootstrap/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml index 5322f0792147a..71c56c4e85e08 100644 --- a/src/bootstrap/Cargo.toml +++ b/src/bootstrap/Cargo.toml @@ -65,7 +65,7 @@ xz2 = "0.1" # Dependencies needed by the build-metrics feature sysinfo = { version = "0.33.0", default-features = false, optional = true, features = ["system"] } -# Dependencies needed by the `logging` feature +# Dependencies needed by the `tracing` feature tracing = { version = "0.1", optional = true, features = ["attributes"] } tracing-subscriber = { version = "0.3", optional = true, features = ["env-filter", "fmt", "registry", "std"] } tracing-tree = { version = "0.4.0", optional = true } From aa14931503ba593b58901d9a2125a53e755e671e Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Mon, 13 Jan 2025 20:50:57 +0000 Subject: [PATCH 13/14] llvm: Allow sized-word rather than ymmword in tests llvm/llvm-project#122530 changes LLVM to use sized-word rather than ymmword for scatter gather pointers. While this will not always be qword, it is for these two tests. --- tests/assembly/simd-intrinsic-gather.rs | 2 +- tests/assembly/simd-intrinsic-scatter.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/assembly/simd-intrinsic-gather.rs b/tests/assembly/simd-intrinsic-gather.rs index 2cbb6cfbb50d9..d2b5a1507f0c8 100644 --- a/tests/assembly/simd-intrinsic-gather.rs +++ b/tests/assembly/simd-intrinsic-gather.rs @@ -38,6 +38,6 @@ pub unsafe extern "C" fn gather_f64x4(mask: m64x4, ptrs: pf64x4) -> f64x4 { // x86-avx512: vpsllq ymm0, ymm0, 63 // x86-avx512-NEXT: vpmovq2m k1, ymm0 // x86-avx512-NEXT: vpxor xmm0, xmm0, xmm0 - // x86-avx512-NEXT: vgatherqpd ymm0 {k1}, ymmword ptr [1*ymm1] + // x86-avx512-NEXT: vgatherqpd ymm0 {k1}, {{(ymmword)|(qword)}} ptr [1*ymm1] simd_gather(f64x4([0_f64, 0_f64, 0_f64, 0_f64]), ptrs, mask) } diff --git a/tests/assembly/simd-intrinsic-scatter.rs b/tests/assembly/simd-intrinsic-scatter.rs index 679972d9b86f7..f7e08e33faaf8 100644 --- a/tests/assembly/simd-intrinsic-scatter.rs +++ b/tests/assembly/simd-intrinsic-scatter.rs @@ -34,6 +34,6 @@ extern "rust-intrinsic" { pub unsafe extern "C" fn scatter_f64x4(values: f64x4, ptrs: pf64x4, mask: m64x4) { // x86-avx512: vpsllq ymm2, ymm2, 63 // x86-avx512-NEXT: vpmovq2m k1, ymm2 - // x86-avx512-NEXT: vscatterqpd ymmword ptr [1*ymm1] {k1}, ymm0 + // x86-avx512-NEXT: vscatterqpd {{(ymmword)|(qword)}} ptr [1*ymm1] {k1}, ymm0 simd_scatter(values, ptrs, mask) } From 4d0a83800138be8e8515d200e58d32ec02351f11 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Mon, 13 Jan 2025 20:08:52 +0100 Subject: [PATCH 14/14] Fix emscripten-wasm-eh with unwind=abort If we build the standard library with wasm-eh then we need to link with `-fwasm-exceptions` even if we compile with `panic=abort` Without this change, linking a `panic=abort` crate fails with: `undefined symbol: __cpp_exception`. Followup to #131830. --- compiler/rustc_codegen_ssa/src/back/link.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index e4b3ad198018f..df35b5e8426f1 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -2451,10 +2451,10 @@ fn add_order_independent_options( } if sess.target.os == "emscripten" { - cmd.cc_arg(if sess.panic_strategy() == PanicStrategy::Abort { - "-sDISABLE_EXCEPTION_CATCHING=1" - } else if sess.opts.unstable_opts.emscripten_wasm_eh { + cmd.cc_arg(if sess.opts.unstable_opts.emscripten_wasm_eh { "-fwasm-exceptions" + } else if sess.panic_strategy() == PanicStrategy::Abort { + "-sDISABLE_EXCEPTION_CATCHING=1" } else { "-sDISABLE_EXCEPTION_CATCHING=0" });