Skip to content

Remove "consider specifying this binding's type" when reference differs in mutability #115648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 57 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use hir::ExprKind;
use hir::def::{DefKind, Res};
use hir::{
BindingAnnotation, Expr, ExprKind, FnDecl, FnRetTy, FnSig, Item, ItemKind, Pat, PatKind, Path,
QPath, TyKind,
};
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
use rustc_hir as hir;
use rustc_hir::intravisit::Visitor;
Expand Down Expand Up @@ -854,7 +858,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
// In the future, attempt in all path but initially for RHS of for_loop
fn suggest_similar_mut_method_for_for_loop(&self, err: &mut Diagnostic, span: Span) {
use hir::{
BorrowKind, Expr,
BorrowKind,
ExprKind::{AddrOf, Block, Call, MethodCall},
};

Expand Down Expand Up @@ -1216,6 +1220,56 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
if let Some(hir_id) = hir_id
&& let Some(hir::Node::Local(local)) = hir_map.find(hir_id)
Comment on lines 1220 to 1221
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the added checks here don't cover this else case. Which can have the same problems?

{

let initial_expression = match local.init.map(|init| init.kind) {
Some(hir::ExprKind::Path(hir::QPath::Resolved(_, hir::Path{res: Res::Local(id), ..} ))) => hir_map.find(*id),
Some(hir::ExprKind::Call(Expr { kind: ExprKind::Path(QPath::Resolved(_, Path {res: Res::Def(DefKind::Fn, defid ) , ..} ), ..), ..}, .. )) => {
let Some(local_id) = defid.as_local() else {
todo!("What do we do here?")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have a fallback case that just has a more lax "you may be able to change the mutability here". I'm not sure if the binding info is available cross-crates.

};

hir_map.find_by_def_id(local_id)
}

_ => None
};

let initial_expression_mutability = match initial_expression {

Some(Node::Pat(Pat {
kind: PatKind::Binding(BindingAnnotation(_, mut_ty), ..),
..
})) => *mut_ty,
Some(Node::Item(Item {
kind:
ItemKind::Fn(
FnSig {
decl:
FnDecl {
output:
FnRetTy::Return(hir::Ty {
kind: TyKind::Ref(_, mut_ty),
..
}),
..
},
..
},
..,
),
..
})) => mut_ty.mutbl,
_ => Mutability::Mut, // TODO: this probably is not correct handling of this case.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, it's okay. Just leave a note that other cases might not be covered.

};

// If the inital value of an expression is not mutable then suggesting that the user
// changes the type of the expression to be mutable is incorrect is it will not help.
// TODO: Could we provide an alternative suggestions around altering the mutability
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine to leave as a FIXME, or just remove.

// of the inital value.
if initial_expression_mutability.is_not() {
return;
}

let (changing, span, sugg) = match local.ty {
Some(ty) => ("changing", ty.span, message),
None => (
Expand All @@ -1224,6 +1278,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
format!(": {message}"),
),
};
// FOUND IT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??

err.span_suggestion_verbose(
span,
format!("consider {changing} this binding's type"),
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/issue-113767.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn get() -> &'static Vec<i32> {
todo!()
}

fn main() {
let x = get();
x.push(1); //~ ERROR cannot borrow `*x` as mutable, as it is behind a `&` reference [E0596]
}
9 changes: 9 additions & 0 deletions tests/ui/issue-113767.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0596]: cannot borrow `*x` as mutable, as it is behind a `&` reference
--> $DIR/issue-113767.rs:7:5
|
LL | x.push(1);
| ^ `x` is a `&` reference, so the data it refers to cannot be borrowed as mutable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0596`.