Skip to content

Commit d18b4bb

Browse files
Aaron1011Aaron1011
committed
Note when a a move/borrow error is caused by a deref coercion
Fixes #73268 When a deref coercion occurs, we may end up with a move error if the base value has been partially moved out of. However, we do not indicate anywhere that a deref coercion is occuring, resulting in an error message with a confusing span. This PR adds an explicit note to move errors when a deref coercion is involved. We mention the name of the type that the deref-coercion resolved to, as well as the `Deref::Target` associated type being used.
1 parent a1947b3 commit d18b4bb

File tree

23 files changed

+250
-54
lines changed

23 files changed

+250
-54
lines changed

‎compiler/rustc_middle/src/ty/adjustment.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use rustc_hir as hir;
44
use rustc_hir::def_id::DefId;
55
use rustc_hir::lang_items::LangItem;
66
use rustc_macros::HashStable;
7+
use rustc_span::Span;
78

89
#[derive(Clone, Copy, Debug, PartialEq, Eq, TyEncodable, TyDecodable, HashStable)]
910
pub enum PointerCast {
@@ -113,6 +114,9 @@ pub enum Adjust<'tcx> {
113114
pub struct OverloadedDeref<'tcx> {
114115
pub region: ty::Region<'tcx>,
115116
pub mutbl: hir::Mutability,
117+
/// The `Span` associated with the field access or method call
118+
/// that triggered this overloaded deref.
119+
pub span: Span,
116120
}
117121

118122
impl<'tcx> OverloadedDeref<'tcx> {

‎compiler/rustc_middle/src/ty/structural_impls.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,8 +606,11 @@ impl<'a, 'tcx> Lift<'tcx> for ty::adjustment::Adjust<'a> {
606606
impl<'a, 'tcx> Lift<'tcx> for ty::adjustment::OverloadedDeref<'a> {
607607
type Lifted = ty::adjustment::OverloadedDeref<'tcx>;
608608
fn lift_to_tcx(&self, tcx: TyCtxt<'tcx>) -> Option<Self::Lifted> {
609-
tcx.lift(&self.region)
610-
.map(|region| ty::adjustment::OverloadedDeref { region, mutbl: self.mutbl })
609+
tcx.lift(&self.region).map(|region| ty::adjustment::OverloadedDeref {
610+
region,
611+
mutbl: self.mutbl,
612+
span: self.span,
613+
})
611614
}
612615
}
613616

‎compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
6666
let span = use_spans.args_or_use();
6767

6868
let move_site_vec = self.get_moved_indexes(location, mpi);
69-
debug!("report_use_of_moved_or_uninitialized: move_site_vec={:?}", move_site_vec);
69+
debug!(
70+
"report_use_of_moved_or_uninitialized: move_site_vec={:?} use_spans={:?}",
71+
move_site_vec, use_spans
72+
);
7073
let move_out_indices: Vec<_> =
7174
move_site_vec.iter().map(|move_site| move_site.moi).collect();
7275

@@ -229,6 +232,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
229232
);
230233
}
231234
}
235+
// Deref::deref takes &self, which cannot cause a move
236+
FnSelfUseKind::DerefCoercion { .. } => unreachable!(),
232237
}
233238
} else {
234239
err.span_label(
@@ -355,6 +360,20 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
355360
self.note_type_does_not_implement_copy(&mut err, &note_msg, ty, span, partial_str);
356361
}
357362

363+
if let UseSpans::FnSelfUse {
364+
kind: FnSelfUseKind::DerefCoercion { deref_target, deref_target_ty },
365+
..
366+
} = use_spans
367+
{
368+
err.note(&format!(
369+
"{} occurs due to deref coercion to `{}`",
370+
desired_action.as_noun(),
371+
deref_target_ty
372+
));
373+
374+
err.span_note(deref_target, "deref defined here");
375+
}
376+
358377
if let Some((_, mut old_err)) =
359378
self.move_error_reported.insert(move_out_indices, (used_place, err))
360379
{
@@ -945,7 +964,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
945964
name: &str,
946965
borrow: &BorrowData<'tcx>,
947966
drop_span: Span,
948-
borrow_spans: UseSpans,
967+
borrow_spans: UseSpans<'tcx>,
949968
explanation: BorrowExplanation,
950969
) -> DiagnosticBuilder<'cx> {
951970
debug!(
@@ -1146,7 +1165,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
11461165
location: Location,
11471166
borrow: &BorrowData<'tcx>,
11481167
drop_span: Span,
1149-
borrow_spans: UseSpans,
1168+
borrow_spans: UseSpans<'tcx>,
11501169
proper_span: Span,
11511170
explanation: BorrowExplanation,
11521171
) -> DiagnosticBuilder<'cx> {
@@ -1274,7 +1293,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
12741293

12751294
fn report_escaping_closure_capture(
12761295
&mut self,
1277-
use_span: UseSpans,
1296+
use_span: UseSpans<'tcx>,
12781297
var_span: Span,
12791298
fr_name: &RegionName,
12801299
category: ConstraintCategory,

‎compiler/rustc_mir/src/borrow_check/diagnostics/explain_borrow.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
501501
fn later_use_kind(
502502
&self,
503503
borrow: &BorrowData<'tcx>,
504-
use_spans: UseSpans,
504+
use_spans: UseSpans<'tcx>,
505505
location: Location,
506506
) -> (LaterUseKind, Span) {
507507
match use_spans {

‎compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use rustc_middle::mir::{
1111
PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind,
1212
};
1313
use rustc_middle::ty::print::Print;
14-
use rustc_middle::ty::{self, DefIdTree, Ty, TyCtxt};
14+
use rustc_middle::ty::{self, DefIdTree, Instance, Ty, TyCtxt};
1515
use rustc_span::{
1616
hygiene::{DesugaringKind, ForLoopLoc},
1717
symbol::sym,
@@ -538,7 +538,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
538538

539539
/// The span(s) associated to a use of a place.
540540
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
541-
pub(super) enum UseSpans {
541+
pub(super) enum UseSpans<'tcx> {
542542
/// The access is caused by capturing a variable for a closure.
543543
ClosureUse {
544544
/// This is true if the captured variable was from a generator.
@@ -558,7 +558,7 @@ pub(super) enum UseSpans {
558558
fn_call_span: Span,
559559
/// The definition span of the method being called
560560
fn_span: Span,
561-
kind: FnSelfUseKind,
561+
kind: FnSelfUseKind<'tcx>,
562562
},
563563
/// This access is caused by a `match` or `if let` pattern.
564564
PatUse(Span),
@@ -567,31 +567,44 @@ pub(super) enum UseSpans {
567567
}
568568

569569
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
570-
pub(super) enum FnSelfUseKind {
570+
pub(super) enum FnSelfUseKind<'tcx> {
571571
/// A normal method call of the form `receiver.foo(a, b, c)`
572572
Normal { self_arg: Ident, implicit_into_iter: bool },
573573
/// A call to `FnOnce::call_once`, desugared from `my_closure(a, b, c)`
574574
FnOnceCall,
575575
/// A call to an operator trait, desuraged from operator syntax (e.g. `a << b`)
576576
Operator { self_arg: Ident },
577+
DerefCoercion {
578+
/// The `Span` of the `Target` associated type
579+
/// in the `Deref` impl we are using.
580+
deref_target: Span,
581+
/// The type `T::Deref` we are dereferencing to
582+
deref_target_ty: Ty<'tcx>,
583+
},
577584
}
578585

579-
impl UseSpans {
586+
impl UseSpans<'_> {
580587
pub(super) fn args_or_use(self) -> Span {
581588
match self {
582589
UseSpans::ClosureUse { args_span: span, .. }
583590
| UseSpans::PatUse(span)
584-
| UseSpans::FnSelfUse { var_span: span, .. }
585591
| UseSpans::OtherUse(span) => span,
592+
UseSpans::FnSelfUse {
593+
fn_call_span, kind: FnSelfUseKind::DerefCoercion { .. }, ..
594+
} => fn_call_span,
595+
UseSpans::FnSelfUse { var_span, .. } => var_span,
586596
}
587597
}
588598

589599
pub(super) fn var_or_use(self) -> Span {
590600
match self {
591601
UseSpans::ClosureUse { var_span: span, .. }
592602
| UseSpans::PatUse(span)
593-
| UseSpans::FnSelfUse { var_span: span, .. }
594603
| UseSpans::OtherUse(span) => span,
604+
UseSpans::FnSelfUse {
605+
fn_call_span, kind: FnSelfUseKind::DerefCoercion { .. }, ..
606+
} => fn_call_span,
607+
UseSpans::FnSelfUse { var_span, .. } => var_span,
595608
}
596609
}
597610

@@ -754,7 +767,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
754767
&self,
755768
moved_place: PlaceRef<'tcx>, // Could also be an upvar.
756769
location: Location,
757-
) -> UseSpans {
770+
) -> UseSpans<'tcx> {
758771
use self::UseSpans::*;
759772

760773
let stmt = match self.body[location.block].statements.get(location.statement_index) {
@@ -809,36 +822,64 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
809822
kind: TerminatorKind::Call { fn_span, from_hir_call, .. }, ..
810823
}) = &self.body[location.block].terminator
811824
{
812-
let method_did = if let Some(method_did) =
825+
let (method_did, method_substs) = if let Some(info) =
813826
crate::util::find_self_call(self.infcx.tcx, &self.body, target_temp, location.block)
814827
{
815-
method_did
828+
info
816829
} else {
817830
return normal_ret;
818831
};
819832

820833
let tcx = self.infcx.tcx;
821-
822834
let parent = tcx.parent(method_did);
823835
let is_fn_once = parent == tcx.lang_items().fn_once_trait();
824836
let is_operator = !from_hir_call
825837
&& parent.map_or(false, |p| tcx.lang_items().group(LangItemGroup::Op).contains(&p));
838+
let is_deref = !from_hir_call && tcx.is_diagnostic_item(sym::deref_method, method_did);
826839
let fn_call_span = *fn_span;
827840

828841
let self_arg = tcx.fn_arg_names(method_did)[0];
829842

843+
debug!(
844+
"terminator = {:?} from_hir_call={:?}",
845+
self.body[location.block].terminator, from_hir_call
846+
);
847+
848+
// Check for a 'special' use of 'self' -
849+
// an FnOnce call, an operator (e.g. `<<`), or a
850+
// deref coercion.
830851
let kind = if is_fn_once {
831-
FnSelfUseKind::FnOnceCall
852+
Some(FnSelfUseKind::FnOnceCall)
832853
} else if is_operator {
833-
FnSelfUseKind::Operator { self_arg }
854+
Some(FnSelfUseKind::Operator { self_arg })
855+
} else if is_deref {
856+
let deref_target =
857+
tcx.get_diagnostic_item(sym::deref_target).and_then(|deref_target| {
858+
Instance::resolve(tcx, self.param_env, deref_target, method_substs)
859+
.transpose()
860+
});
861+
if let Some(Ok(instance)) = deref_target {
862+
let deref_target_ty = instance.ty(tcx, self.param_env);
863+
Some(FnSelfUseKind::DerefCoercion {
864+
deref_target: tcx.def_span(instance.def_id()),
865+
deref_target_ty,
866+
})
867+
} else {
868+
None
869+
}
834870
} else {
871+
None
872+
};
873+
874+
let kind = kind.unwrap_or_else(|| {
875+
// This isn't a 'special' use of `self`
835876
debug!("move_spans: method_did={:?}, fn_call_span={:?}", method_did, fn_call_span);
836877
let implicit_into_iter = matches!(
837878
fn_call_span.desugaring_kind(),
838879
Some(DesugaringKind::ForLoop(ForLoopLoc::IntoIter))
839880
);
840881
FnSelfUseKind::Normal { self_arg, implicit_into_iter }
841-
};
882+
});
842883

843884
return FnSelfUse {
844885
var_span: stmt.source_info.span,
@@ -859,7 +900,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
859900
/// and its usage of the local assigned at `location`.
860901
/// This is done by searching in statements succeeding `location`
861902
/// and originating from `maybe_closure_span`.
862-
pub(super) fn borrow_spans(&self, use_span: Span, location: Location) -> UseSpans {
903+
pub(super) fn borrow_spans(&self, use_span: Span, location: Location) -> UseSpans<'tcx> {
863904
use self::UseSpans::*;
864905
debug!("borrow_spans: use_span={:?} location={:?}", use_span, location);
865906

@@ -963,7 +1004,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
9631004

9641005
/// Helper to retrieve span(s) of given borrow from the current MIR
9651006
/// representation
966-
pub(super) fn retrieve_borrow_spans(&self, borrow: &BorrowData<'_>) -> UseSpans {
1007+
pub(super) fn retrieve_borrow_spans(&self, borrow: &BorrowData<'_>) -> UseSpans<'tcx> {
9671008
let span = self.body.source_info(borrow.reserve_location).span;
9681009
self.borrow_spans(span, borrow.reserve_location)
9691010
}

‎compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ enum GroupedMoveError<'tcx> {
4747
// Everything that isn't from pattern matching.
4848
OtherIllegalMove {
4949
original_path: Place<'tcx>,
50-
use_spans: UseSpans,
50+
use_spans: UseSpans<'tcx>,
5151
kind: IllegalMoveOriginKind<'tcx>,
5252
},
5353
}
@@ -222,7 +222,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
222222
let (mut err, err_span) = {
223223
let (span, use_spans, original_path, kind): (
224224
Span,
225-
Option<UseSpans>,
225+
Option<UseSpans<'tcx>>,
226226
Place<'tcx>,
227227
&IllegalMoveOriginKind<'_>,
228228
) = match error {
@@ -291,7 +291,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
291291
move_place: Place<'tcx>,
292292
deref_target_place: Place<'tcx>,
293293
span: Span,
294-
use_spans: Option<UseSpans>,
294+
use_spans: Option<UseSpans<'tcx>>,
295295
) -> DiagnosticBuilder<'a> {
296296
// Inspect the type of the content behind the
297297
// borrow to provide feedback about why this

‎compiler/rustc_mir/src/borrow_check/mod.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use rustc_middle::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind
1717
use rustc_middle::mir::{Field, ProjectionElem, Promoted, Rvalue, Statement, StatementKind};
1818
use rustc_middle::mir::{InlineAsmOperand, Terminator, TerminatorKind};
1919
use rustc_middle::ty::query::Providers;
20-
use rustc_middle::ty::{self, InstanceDef, RegionVid, TyCtxt};
20+
use rustc_middle::ty::{self, InstanceDef, ParamEnv, RegionVid, TyCtxt};
2121
use rustc_session::lint::builtin::{MUTABLE_BORROW_RESERVATION_CONFLICT, UNUSED_MUT};
2222
use rustc_span::{Span, Symbol, DUMMY_SP};
2323

@@ -287,6 +287,7 @@ fn do_mir_borrowck<'a, 'tcx>(
287287
if let Err((move_data, move_errors)) = move_data_results {
288288
let mut promoted_mbcx = MirBorrowckCtxt {
289289
infcx,
290+
param_env,
290291
body: promoted_body,
291292
mir_def_id: def.did,
292293
move_data: &move_data,
@@ -320,6 +321,7 @@ fn do_mir_borrowck<'a, 'tcx>(
320321

321322
let mut mbcx = MirBorrowckCtxt {
322323
infcx,
324+
param_env,
323325
body,
324326
mir_def_id: def.did,
325327
move_data: &mdpe.move_data,
@@ -473,6 +475,7 @@ fn do_mir_borrowck<'a, 'tcx>(
473475

474476
crate struct MirBorrowckCtxt<'cx, 'tcx> {
475477
crate infcx: &'cx InferCtxt<'cx, 'tcx>,
478+
param_env: ParamEnv<'tcx>,
476479
body: &'cx Body<'tcx>,
477480
mir_def_id: LocalDefId,
478481
move_data: &'cx MoveData<'tcx>,

‎compiler/rustc_mir/src/transform/check_const_item_mutation.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ConstMutationChecker<'a, 'tcx> {
101101
.note("each usage of a `const` item creates a new temporary")
102102
.note("the mutable reference will refer to this temporary, not the original `const` item");
103103

104-
if let Some(method_did) = method_did {
104+
if let Some((method_did, _substs)) = method_did {
105105
lint.span_note(self.tcx.def_span(method_did), "mutable reference created due to call to this method");
106106
}
107107

‎compiler/rustc_mir/src/util/find_self_call.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,31 @@
11
use rustc_middle::mir::*;
2+
use rustc_middle::ty::subst::SubstsRef;
23
use rustc_middle::ty::{self, TyCtxt};
34
use rustc_span::def_id::DefId;
45

56
/// Checks if the specified `local` is used as the `self` prameter of a method call
67
/// in the provided `BasicBlock`. If it is, then the `DefId` of the called method is
78
/// returned.
8-
pub fn find_self_call(
9-
tcx: TyCtxt<'_>,
10-
body: &Body<'_>,
9+
pub fn find_self_call<'tcx>(
10+
tcx: TyCtxt<'tcx>,
11+
body: &Body<'tcx>,
1112
local: Local,
1213
block: BasicBlock,
13-
) -> Option<DefId> {
14+
) -> Option<(DefId, SubstsRef<'tcx>)> {
1415
debug!("find_self_call(local={:?}): terminator={:?}", local, &body[block].terminator);
1516
if let Some(Terminator { kind: TerminatorKind::Call { func, args, .. }, .. }) =
1617
&body[block].terminator
1718
{
1819
debug!("find_self_call: func={:?}", func);
1920
if let Operand::Constant(box Constant { literal: ty::Const { ty, .. }, .. }) = func {
20-
if let ty::FnDef(def_id, _) = *ty.kind() {
21+
if let ty::FnDef(def_id, substs) = *ty.kind() {
2122
if let Some(ty::AssocItem { fn_has_self_parameter: true, .. }) =
2223
tcx.opt_associated_item(def_id)
2324
{
2425
debug!("find_self_call: args={:?}", args);
2526
if let [Operand::Move(self_place) | Operand::Copy(self_place), ..] = **args {
2627
if self_place.as_local() == Some(local) {
27-
return Some(def_id);
28+
return Some((def_id, substs));
2829
}
2930
}
3031
}

0 commit comments

Comments
 (0)