Skip to content

Commit ec30876

Browse files
borsbors
committedJul 13, 2019
Auto merge of #62468 - rust-lang:mutable-overloaded-operators, r=estebank
Improve diagnostics for invalid mutation through overloaded operators Closes #58864 Closes #52941 Closes #57839
2 parents 6b468c6 + 38306ad commit ec30876

13 files changed

+282
-269
lines changed
 

Diff for: ‎src/librustc_mir/borrow_check/error_reporting.rs

+151-2
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ use rustc::hir::def_id::DefId;
44
use rustc::mir::{
55
AggregateKind, Constant, Field, Local, LocalKind, Location, Operand,
66
Place, PlaceBase, ProjectionElem, Rvalue, Statement, StatementKind, Static,
7-
StaticKind, TerminatorKind,
7+
StaticKind, Terminator, TerminatorKind,
88
};
9-
use rustc::ty::{self, DefIdTree, Ty};
9+
use rustc::ty::{self, DefIdTree, Ty, TyCtxt};
1010
use rustc::ty::layout::VariantIdx;
1111
use rustc::ty::print::Print;
1212
use rustc_errors::DiagnosticBuilder;
@@ -15,6 +15,7 @@ use syntax::symbol::sym;
1515

1616
use super::borrow_set::BorrowData;
1717
use super::{MirBorrowckCtxt};
18+
use crate::dataflow::move_paths::{InitLocation, LookupResult};
1819

1920
pub(super) struct IncludingDowncast(pub(super) bool);
2021

@@ -401,6 +402,70 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
401402
err.note(&message);
402403
}
403404
}
405+
406+
pub(super) fn borrowed_content_source(
407+
&self,
408+
deref_base: &Place<'tcx>,
409+
) -> BorrowedContentSource<'tcx> {
410+
let tcx = self.infcx.tcx;
411+
412+
// Look up the provided place and work out the move path index for it,
413+
// we'll use this to check whether it was originally from an overloaded
414+
// operator.
415+
match self.move_data.rev_lookup.find(deref_base) {
416+
LookupResult::Exact(mpi) | LookupResult::Parent(Some(mpi)) => {
417+
debug!("borrowed_content_source: mpi={:?}", mpi);
418+
419+
for i in &self.move_data.init_path_map[mpi] {
420+
let init = &self.move_data.inits[*i];
421+
debug!("borrowed_content_source: init={:?}", init);
422+
// We're only interested in statements that initialized a value, not the
423+
// initializations from arguments.
424+
let loc = match init.location {
425+
InitLocation::Statement(stmt) => stmt,
426+
_ => continue,
427+
};
428+
429+
let bbd = &self.body[loc.block];
430+
let is_terminator = bbd.statements.len() == loc.statement_index;
431+
debug!(
432+
"borrowed_content_source: loc={:?} is_terminator={:?}",
433+
loc,
434+
is_terminator,
435+
);
436+
if !is_terminator {
437+
continue;
438+
} else if let Some(Terminator {
439+
kind: TerminatorKind::Call {
440+
ref func,
441+
from_hir_call: false,
442+
..
443+
},
444+
..
445+
}) = bbd.terminator {
446+
if let Some(source)
447+
= BorrowedContentSource::from_call(func.ty(self.body, tcx), tcx)
448+
{
449+
return source;
450+
}
451+
}
452+
}
453+
}
454+
// Base is a `static` so won't be from an overloaded operator
455+
_ => (),
456+
};
457+
458+
// If we didn't find an overloaded deref or index, then assume it's a
459+
// built in deref and check the type of the base.
460+
let base_ty = deref_base.ty(self.body, tcx).ty;
461+
if base_ty.is_unsafe_ptr() {
462+
BorrowedContentSource::DerefRawPointer
463+
} else if base_ty.is_mutable_pointer() {
464+
BorrowedContentSource::DerefMutableRef
465+
} else {
466+
BorrowedContentSource::DerefSharedRef
467+
}
468+
}
404469
}
405470

406471
impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
@@ -547,6 +612,90 @@ impl UseSpans {
547612
}
548613
}
549614

615+
pub(super) enum BorrowedContentSource<'tcx> {
616+
DerefRawPointer,
617+
DerefMutableRef,
618+
DerefSharedRef,
619+
OverloadedDeref(Ty<'tcx>),
620+
OverloadedIndex(Ty<'tcx>),
621+
}
622+
623+
impl BorrowedContentSource<'tcx> {
624+
pub(super) fn describe_for_unnamed_place(&self) -> String {
625+
match *self {
626+
BorrowedContentSource::DerefRawPointer => format!("a raw pointer"),
627+
BorrowedContentSource::DerefSharedRef => format!("a shared reference"),
628+
BorrowedContentSource::DerefMutableRef => {
629+
format!("a mutable reference")
630+
}
631+
BorrowedContentSource::OverloadedDeref(ty) => {
632+
if ty.is_rc() {
633+
format!("an `Rc`")
634+
} else if ty.is_arc() {
635+
format!("an `Arc`")
636+
} else {
637+
format!("dereference of `{}`", ty)
638+
}
639+
}
640+
BorrowedContentSource::OverloadedIndex(ty) => format!("index of `{}`", ty),
641+
}
642+
}
643+
644+
pub(super) fn describe_for_named_place(&self) -> Option<&'static str> {
645+
match *self {
646+
BorrowedContentSource::DerefRawPointer => Some("raw pointer"),
647+
BorrowedContentSource::DerefSharedRef => Some("shared reference"),
648+
BorrowedContentSource::DerefMutableRef => Some("mutable reference"),
649+
// Overloaded deref and index operators should be evaluated into a
650+
// temporary. So we don't need a description here.
651+
BorrowedContentSource::OverloadedDeref(_)
652+
| BorrowedContentSource::OverloadedIndex(_) => None
653+
}
654+
}
655+
656+
pub(super) fn describe_for_immutable_place(&self) -> String {
657+
match *self {
658+
BorrowedContentSource::DerefRawPointer => format!("a `*const` pointer"),
659+
BorrowedContentSource::DerefSharedRef => format!("a `&` reference"),
660+
BorrowedContentSource::DerefMutableRef => {
661+
bug!("describe_for_immutable_place: DerefMutableRef isn't immutable")
662+
},
663+
BorrowedContentSource::OverloadedDeref(ty) => {
664+
if ty.is_rc() {
665+
format!("an `Rc`")
666+
} else if ty.is_arc() {
667+
format!("an `Arc`")
668+
} else {
669+
format!("a dereference of `{}`", ty)
670+
}
671+
}
672+
BorrowedContentSource::OverloadedIndex(ty) => format!("an index of `{}`", ty),
673+
}
674+
}
675+
676+
fn from_call(func: Ty<'tcx>, tcx: TyCtxt<'tcx>) -> Option<Self> {
677+
match func.sty {
678+
ty::FnDef(def_id, substs) => {
679+
let trait_id = tcx.trait_of_item(def_id)?;
680+
681+
let lang_items = tcx.lang_items();
682+
if Some(trait_id) == lang_items.deref_trait()
683+
|| Some(trait_id) == lang_items.deref_mut_trait()
684+
{
685+
Some(BorrowedContentSource::OverloadedDeref(substs.type_at(0)))
686+
} else if Some(trait_id) == lang_items.index_trait()
687+
|| Some(trait_id) == lang_items.index_mut_trait()
688+
{
689+
Some(BorrowedContentSource::OverloadedIndex(substs.type_at(0)))
690+
} else {
691+
None
692+
}
693+
}
694+
_ => None,
695+
}
696+
}
697+
}
698+
550699
impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
551700
/// Finds the spans associated to a move or copy of move_place at location.
552701
pub(super) fn move_spans(

Diff for: ‎src/librustc_mir/borrow_check/move_errors.rs

+2-127
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
use core::unicode::property::Pattern_White_Space;
22

33
use rustc::mir::*;
4-
use rustc::ty::{self, Ty, TyCtxt};
4+
use rustc::ty;
55
use rustc_errors::{DiagnosticBuilder,Applicability};
66
use syntax_pos::Span;
77

88
use crate::borrow_check::MirBorrowckCtxt;
99
use crate::borrow_check::prefixes::PrefixSet;
1010
use crate::borrow_check::error_reporting::UseSpans;
1111
use crate::dataflow::move_paths::{
12-
IllegalMoveOrigin, IllegalMoveOriginKind, InitLocation,
12+
IllegalMoveOrigin, IllegalMoveOriginKind,
1313
LookupResult, MoveError, MovePathIndex,
1414
};
1515
use crate::util::borrowck_errors::{BorrowckErrors, Origin};
@@ -55,70 +55,6 @@ enum GroupedMoveError<'tcx> {
5555
},
5656
}
5757

58-
enum BorrowedContentSource<'tcx> {
59-
DerefRawPointer,
60-
DerefMutableRef,
61-
DerefSharedRef,
62-
OverloadedDeref(Ty<'tcx>),
63-
OverloadedIndex(Ty<'tcx>),
64-
}
65-
66-
impl BorrowedContentSource<'tcx> {
67-
fn describe_for_unnamed_place(&self) -> String {
68-
match *self {
69-
BorrowedContentSource::DerefRawPointer => format!("a raw pointer"),
70-
BorrowedContentSource::DerefSharedRef => format!("a shared reference"),
71-
BorrowedContentSource::DerefMutableRef => {
72-
format!("a mutable reference")
73-
}
74-
BorrowedContentSource::OverloadedDeref(ty) => {
75-
if ty.is_rc() {
76-
format!("an `Rc`")
77-
} else if ty.is_arc() {
78-
format!("an `Arc`")
79-
} else {
80-
format!("dereference of `{}`", ty)
81-
}
82-
}
83-
BorrowedContentSource::OverloadedIndex(ty) => format!("index of `{}`", ty),
84-
}
85-
}
86-
87-
fn describe_for_named_place(&self) -> Option<&'static str> {
88-
match *self {
89-
BorrowedContentSource::DerefRawPointer => Some("raw pointer"),
90-
BorrowedContentSource::DerefSharedRef => Some("shared reference"),
91-
BorrowedContentSource::DerefMutableRef => Some("mutable reference"),
92-
// Overloaded deref and index operators should be evaluated into a
93-
// temporary. So we don't need a description here.
94-
BorrowedContentSource::OverloadedDeref(_)
95-
| BorrowedContentSource::OverloadedIndex(_) => None
96-
}
97-
}
98-
99-
fn from_call(func: Ty<'tcx>, tcx: TyCtxt<'tcx>) -> Option<Self> {
100-
match func.sty {
101-
ty::FnDef(def_id, substs) => {
102-
let trait_id = tcx.trait_of_item(def_id)?;
103-
104-
let lang_items = tcx.lang_items();
105-
if Some(trait_id) == lang_items.deref_trait()
106-
|| Some(trait_id) == lang_items.deref_mut_trait()
107-
{
108-
Some(BorrowedContentSource::OverloadedDeref(substs.type_at(0)))
109-
} else if Some(trait_id) == lang_items.index_trait()
110-
|| Some(trait_id) == lang_items.index_mut_trait()
111-
{
112-
Some(BorrowedContentSource::OverloadedIndex(substs.type_at(0)))
113-
} else {
114-
None
115-
}
116-
}
117-
_ => None,
118-
}
119-
}
120-
}
121-
12258
impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
12359
pub(crate) fn report_move_errors(&mut self, move_errors: Vec<(Place<'tcx>, MoveError<'tcx>)>) {
12460
let grouped_errors = self.group_move_errors(move_errors);
@@ -646,65 +582,4 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
646582
);
647583
}
648584
}
649-
650-
fn borrowed_content_source(&self, deref_base: &Place<'tcx>) -> BorrowedContentSource<'tcx> {
651-
let tcx = self.infcx.tcx;
652-
653-
// Look up the provided place and work out the move path index for it,
654-
// we'll use this to check whether it was originally from an overloaded
655-
// operator.
656-
match self.move_data.rev_lookup.find(deref_base) {
657-
LookupResult::Exact(mpi) | LookupResult::Parent(Some(mpi)) => {
658-
debug!("borrowed_content_source: mpi={:?}", mpi);
659-
660-
for i in &self.move_data.init_path_map[mpi] {
661-
let init = &self.move_data.inits[*i];
662-
debug!("borrowed_content_source: init={:?}", init);
663-
// We're only interested in statements that initialized a value, not the
664-
// initializations from arguments.
665-
let loc = match init.location {
666-
InitLocation::Statement(stmt) => stmt,
667-
_ => continue,
668-
};
669-
670-
let bbd = &self.body[loc.block];
671-
let is_terminator = bbd.statements.len() == loc.statement_index;
672-
debug!(
673-
"borrowed_content_source: loc={:?} is_terminator={:?}",
674-
loc,
675-
is_terminator,
676-
);
677-
if !is_terminator {
678-
continue;
679-
} else if let Some(Terminator {
680-
kind: TerminatorKind::Call {
681-
ref func,
682-
from_hir_call: false,
683-
..
684-
},
685-
..
686-
}) = bbd.terminator {
687-
if let Some(source)
688-
= BorrowedContentSource::from_call(func.ty(self.body, tcx), tcx)
689-
{
690-
return source;
691-
}
692-
}
693-
}
694-
}
695-
// Base is a `static` so won't be from an overloaded operator
696-
_ => (),
697-
};
698-
699-
// If we didn't find an overloaded deref or index, then assume it's a
700-
// built in deref and check the type of the base.
701-
let base_ty = deref_base.ty(self.body, tcx).ty;
702-
if base_ty.is_unsafe_ptr() {
703-
BorrowedContentSource::DerefRawPointer
704-
} else if base_ty.is_mutable_pointer() {
705-
BorrowedContentSource::DerefMutableRef
706-
} else {
707-
BorrowedContentSource::DerefSharedRef
708-
}
709-
}
710585
}

0 commit comments

Comments
 (0)
Please sign in to comment.