fix: add deref suggestion for type mismatch binary op #112961
Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
bvanjoi
changed the title
| })) = self.tcx.hir().find_parent(expr.hir_id) && mutability.is_mut() { | ||
| let deref_expr = if expr.hir_id == left.hir_id { right } else { left }; | ||
| let sugg = vec![(deref_expr.span.shrink_to_lo(), "*".to_string())]; | ||
| return Some(( |
There was a problem hiding this comment.
The reason why I didn't think it needed to be wrapped in parentheses is that I thought it would throw an error beforehand.
| let sugg = vec![(deref_expr.span.shrink_to_lo(), "*".to_string())]; | ||
| return Some(( | ||
| sugg, | ||
| format!("consider dereference here"), |
There was a problem hiding this comment.
| format!("consider dereference here"), | |
| format!("consider dereferencing here"), |
Further, I think it'd make sense to use a translatable diagnostic here, so less needs to be migrated later on.
There was a problem hiding this comment.
I think it'd make sense to use a translatable diagnostic here
The return type of suggest_deref_or_ref is a tuple instead of Diagnostic, so I think it would be better to change it in another PR
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| if let Some(hir::Node::Expr(hir::Expr { | ||
| kind: hir::ExprKind::Binary(_, left, right), | ||
| .. | ||
| })) = self.tcx.hir().find_parent(expr.hir_id) && mutability.is_mut() { |
There was a problem hiding this comment.
I don't think this needs special-casing for mutable references.
There was a problem hiding this comment.
Do you mean mutability.is_mut()? It does to prevent this problem:
let a = String::new();
let b = String::new();
let _ = a + b // should suggestion a + &b, rather than *a + bThere was a problem hiding this comment.
Well what we really should be doing is checking whether the obligation Lhs: BinOp<Rhs> holds after dereffing the left-hand side. Not really sure if that's possible here, so I guess maybe give that a try or else I can approve..
There was a problem hiding this comment.
checking whether the obligation Lhs: BinOp holds after dereffing the left-hand side
I haven't found a definitive solution yet. Do you have any thoughts or suggestions?
| if let Some(hir::Node::Expr(hir::Expr { | ||
| kind: hir::ExprKind::Binary(_, left, right), | ||
| .. | ||
| })) = self.tcx.hir().find_parent(expr.hir_id) && mutability.is_mut() { |
There was a problem hiding this comment.
Well what we really should be doing is checking whether the obligation Lhs: BinOp<Rhs> holds after dereffing the left-hand side. Not really sure if that's possible here, so I guess maybe give that a try or else I can approve..
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
@bvanjoi any updates no this? |
I haven't found a suitable check for this yet. We can wait for suggestions from @compiler-errors. |
|
☔ The latest upstream changes (presumably #117482) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bvanjoi FYI: when a PR is ready for review, send a message containing Or if you're not going to continue, please close it. Thank you! |
Fixes #112958
Find the mutable expression in binary operation and add * suggestion before it