Skip to content

Commit aca14f5

Browse files
GTimothyGTimothy
committed
Fix 'assign to data in an index of' collection suggestions
splits the large triple suggestion into three sets them to MaybeIncorrect automatically determines the required/nicest borrowing to use.
1 parent 57f772f commit aca14f5

1 file changed

Lines changed: 135 additions & 32 deletions

File tree

‎compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs‎

Lines changed: 135 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rustc_abi::FieldIdx;
66
use rustc_errors::{Applicability, Diag};
77
use rustc_hir::def_id::DefId;
88
use rustc_hir::intravisit::Visitor;
9-
use rustc_hir::{self as hir, BindingMode, ByRef, Node};
9+
use rustc_hir::{self as hir, BindingMode, ByRef, Expr, Node};
1010
use rustc_middle::bug;
1111
use rustc_middle::hir::place::PlaceBase;
1212
use rustc_middle::mir::visit::PlaceContext;
@@ -669,7 +669,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
669669
err: &'a mut Diag<'infcx>,
670670
ty: Ty<'tcx>,
671671
suggested: bool,
672+
infcx: &'a rustc_infer::infer::InferCtxt<'tcx>,
672673
}
674+
673675
impl<'a, 'infcx, 'tcx> Visitor<'tcx> for SuggestIndexOperatorAlternativeVisitor<'a, 'infcx, 'tcx> {
674676
fn visit_stmt(&mut self, stmt: &'tcx hir::Stmt<'tcx>) {
675677
hir::intravisit::walk_stmt(self, stmt);
@@ -680,60 +682,160 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
680682
return;
681683
}
682684
};
685+
686+
// Because of TypeChecking and indexing, we know: index is &Q
687+
// with K: Eq + Hash + Borrow<Q>,
688+
// with Q: Eq + Hash + ?Sized,
689+
//
690+
// which fulfill the requirements of `get_mut`. If Q=K or Q=&{n}K, the requirements
691+
// of `entry` and `insert` are fulfilled too after dereferencing. If K is not
692+
// copy, a subsequent `clone` call may be needed.
693+
694+
/// Taken straight from https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.peel_hir_ty_refs.html
695+
/// Adapted to mid using https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.Ty.html#method.peel_refs
696+
/// Simplified to counting only
697+
/// Peels off all references on the type. Returns the number of references
698+
/// removed.
699+
fn count_ty_refs<'tcx>(mut ty: Ty<'tcx>) -> usize {
700+
let mut count = 0;
701+
while let ty::Ref(_, inner_ty, _) = ty.kind() {
702+
ty = *inner_ty;
703+
count += 1;
704+
}
705+
count
706+
}
707+
708+
/// Try to strip `n` `&` reference from an expression.
709+
/// If the expression does not have enough leading `&`, return an Error
710+
/// containing a count of the successfull stripped ones and the stripped
711+
/// expression.
712+
fn strip_n_refs<'a, 'b>(
713+
mut expr: &'a Expr<'b>,
714+
n: usize,
715+
) -> Result<&'a Expr<'b>, (usize, &'a Expr<'b>)> {
716+
for count in 0..n {
717+
match expr {
718+
Expr {
719+
kind: ExprKind::AddrOf(hir::BorrowKind::Ref, _, inner),
720+
..
721+
} => expr = inner,
722+
_ => return Err((count, expr)),
723+
}
724+
}
725+
Ok(expr)
726+
}
727+
728+
// we know ty is a map, with a key type at walk distance 2.
729+
let key_ty = self.ty.walk().nth(1).unwrap().expect_ty();
730+
683731
if let hir::ExprKind::Assign(place, rv, _sp) = expr.kind
684732
&& let hir::ExprKind::Index(val, index, _) = place.kind
685733
&& (expr.span == self.assign_span || place.span == self.assign_span)
686734
{
687735
// val[index] = rv;
688-
// ---------- place
689-
self.err.multipart_suggestions(
690-
format!(
691-
"use `.insert()` to insert a value into a `{}`, `.get_mut()` \
692-
to modify it, or the entry API for more flexibility",
693-
self.ty,
694-
),
695-
vec![
736+
let index_ty =
737+
self.infcx.tcx.typeck(val.hir_id.owner.def_id).expr_ty(index);
738+
739+
let count_refs_diff: isize =
740+
count_ty_refs(index_ty) as isize - count_ty_refs(key_ty) as isize;
741+
742+
let (borrowed_prefix, borrowed_index);
743+
744+
// only suggest `insert` an `entry` if index is of type K or &{n}K.
745+
// We use `peel_refs` because borrow lifetimes may differ in both index and
746+
// key. I.e, if they are of the same base type:
747+
if index_ty.peel_refs() == key_ty.peel_refs() {
748+
assert!(
749+
count_refs_diff >= 0,
750+
"compiler bug I think, please report this"
751+
);
752+
// if base type is same, borrowed depth must be exact.
753+
let (deref_prefix, deref_index) =
754+
strip_n_refs(index, count_refs_diff as usize)
755+
.map(|val| ("".to_string(), val))
756+
.unwrap_or_else(|(depth, val)| {
757+
(
758+
"*".repeat(
759+
usize::try_from(count_refs_diff)
760+
.expect("passed assert")
761+
- depth,
762+
),
763+
val,
764+
)
765+
});
766+
767+
self.err.multipart_suggestion(
768+
format!("use `.insert()` to insert a value into a `{}`", self.ty),
696769
vec![
697-
// val.insert(index, rv);
770+
// val.insert({deref_prefix}{deref_index}, rv);
698771
(
699-
val.span.shrink_to_hi().with_hi(index.span.lo()),
700-
".insert(".to_string(),
772+
val.span.shrink_to_hi().with_hi(deref_index.span.lo()),
773+
format!(".insert({deref_prefix}"),
701774
),
702775
(
703-
index.span.shrink_to_hi().with_hi(rv.span.lo()),
776+
deref_index.span.shrink_to_hi().with_hi(rv.span.lo()),
704777
", ".to_string(),
705778
),
706779
(rv.span.shrink_to_hi(), ")".to_string()),
707780
],
781+
Applicability::MaybeIncorrect,
782+
);
783+
self.err.multipart_suggestion(
784+
format!(
785+
"use the entry API to modify a `{}` for more flexibility",
786+
self.ty
787+
),
708788
vec![
709-
// if let Some(v) = val.get_mut(index) { *v = rv; }
710-
(val.span.shrink_to_lo(), "if let Some(val) = ".to_string()),
711-
(
712-
val.span.shrink_to_hi().with_hi(index.span.lo()),
713-
".get_mut(".to_string(),
714-
),
715-
(
716-
index.span.shrink_to_hi().with_hi(place.span.hi()),
717-
") { *val".to_string(),
718-
),
719-
(rv.span.shrink_to_hi(), "; }".to_string()),
720-
],
721-
vec![
722-
// let x = val.entry(index).or_insert(rv);
789+
// let x = val.entry({deref_prefix}{deref_index}).insert_entry(rv);
723790
(val.span.shrink_to_lo(), "let val = ".to_string()),
724791
(
725-
val.span.shrink_to_hi().with_hi(index.span.lo()),
726-
".entry(".to_string(),
792+
val.span.shrink_to_hi().with_hi(deref_index.span.lo()),
793+
format!(".entry({deref_prefix}"),
727794
),
728795
(
729-
index.span.shrink_to_hi().with_hi(rv.span.lo()),
730-
").or_insert(".to_string(),
796+
deref_index.span.shrink_to_hi().with_hi(rv.span.lo()),
797+
").insert_entry(".to_string(),
731798
),
732799
(rv.span.shrink_to_hi(), ")".to_string()),
733800
],
801+
Applicability::MaybeIncorrect,
802+
);
803+
804+
// we can make the next suggestions nicer by stripping as many leading `&` as
805+
// we can, autoderef will do the rest
806+
(borrowed_prefix, borrowed_index) =
807+
match strip_n_refs(index, 0.max(count_refs_diff - 1) as usize) {
808+
Ok(val) => (String::new(), val),
809+
// even if we tried to strip more, we can stop there thanks to
810+
// autoderef
811+
Err((_depth, val)) => (String::new(), val),
812+
};
813+
} else {
814+
(borrowed_prefix, borrowed_index) = (String::new(), index)
815+
}
816+
// in all cases, suggest get_mut because K:Borrow<K> or Q:Borrow<K> as a
817+
// requirement of indexing.
818+
self.err.multipart_suggestion(
819+
format!(
820+
"use `.get_mut()` to modify an existing key in a `{}`",
821+
self.ty,
822+
),
823+
vec![
824+
// if let Some(v) = val.get_mut({borrowed_prefix}{borrowed_index}) { *v = rv; }
825+
(val.span.shrink_to_lo(), "if let Some(val) = ".to_string()),
826+
(
827+
val.span.shrink_to_hi().with_hi(borrowed_index.span.lo()),
828+
format!(".get_mut({borrowed_prefix}"),
829+
),
830+
(
831+
borrowed_index.span.shrink_to_hi().with_hi(place.span.hi()),
832+
") { *val".to_string(),
833+
),
834+
(rv.span.shrink_to_hi(), "; }".to_string()),
734835
],
735-
Applicability::MachineApplicable,
836+
Applicability::MaybeIncorrect,
736837
);
838+
737839
self.suggested = true;
738840
} else if let hir::ExprKind::MethodCall(_path, receiver, _, sp) = expr.kind
739841
&& let hir::ExprKind::Index(val, index, _) = receiver.kind
@@ -769,6 +871,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
769871
err,
770872
ty,
771873
suggested: false,
874+
infcx: self.infcx,
772875
};
773876
v.visit_body(&body);
774877
if !v.suggested {

0 commit comments

Comments
 (0)