Skip to content

Commit

Permalink
review comments and make test run-rustfix
Browse files Browse the repository at this point in the history
  • Loading branch information
estebank committed Jan 11, 2025
1 parent ec98df4 commit 4438b32
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 84 deletions.
138 changes: 64 additions & 74 deletions compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,82 +822,72 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
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 };
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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
//@ 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
//~| ERROR value assigned to `object` is never read
}

fn main() {
let object = Object;
change_object(&object);
change_object2(&object);
let mut object = Object;
change_object(&mut object);
change_object2(&mut object);
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand Down

0 comments on commit 4438b32

Please sign in to comment.