diff options
| author | Esteban Küber <esteban@kuber.com.ar> | 2024-12-31 17:13:11 +0000 |
|---|---|---|
| committer | Esteban Küber <esteban@kuber.com.ar> | 2025-01-11 01:34:23 +0000 |
| commit | c2ae386c851b9f85439375dc788a506e2482ca60 (patch) | |
| tree | 4ad1ade571b635d09670aa0053d8fbd6ac5ae4ba /compiler | |
| parent | 57f9f8f883b2621296549d82669299aecd00cc78 (diff) | |
| download | rust-c2ae386c851b9f85439375dc788a506e2482ca60.tar.gz rust-c2ae386c851b9f85439375dc788a506e2482ca60.zip | |
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.
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_hir_typeck/src/demand.rs | 105 |
1 files changed, 105 insertions, 0 deletions
diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 56f7a2c1150..557e532ca8c 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<TypeError<'tcx>>, + ) -> 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<'_>, |
