From eed311f719ac4cda2f1df2fae5f4ce9404cae135 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 14 Dec 2019 17:43:07 +0100 Subject: [PATCH] add check_borrow_conflicts_in_at_patterns analysis --- src/librustc_mir/hair/pattern/check_match.rs | 134 ++++++++++++++----- 1 file changed, 104 insertions(+), 30 deletions(-) diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index c3768e74385..3f7a17183e7 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -15,6 +15,7 @@ use rustc::ty::{self, Ty, TyCtxt}; use rustc_error_codes::*; use rustc_errors::{Applicability, DiagnosticBuilder}; +use syntax::ast::Mutability; use syntax::feature_gate::feature_err; use syntax_pos::symbol::sym; use syntax_pos::{MultiSpan, Span}; @@ -122,6 +123,7 @@ fn span_e0158(&self, span: Span, text: &str) { impl<'tcx> MatchVisitor<'_, 'tcx> { fn check_patterns(&mut self, has_guard: bool, pat: &Pat) { check_legality_of_move_bindings(self, has_guard, pat); + check_borrow_conflicts_in_at_patterns(self, pat); if !self.tcx.features().bindings_after_at { check_legality_of_bindings_in_at_patterns(self, pat); } @@ -639,44 +641,116 @@ fn check_legality_of_move_bindings(cx: &mut MatchVisitor<'_, '_>, has_guard: boo } } -/// Forbids bindings in `@` patterns. This is necessary for memory safety, -/// because of the way rvalues are handled in the borrow check. (See issue -/// #14587.) -fn check_legality_of_bindings_in_at_patterns(cx: &MatchVisitor<'_, '_>, pat: &Pat) { - AtBindingPatternVisitor { cx, bindings_allowed: true }.visit_pat(pat); -} +fn check_borrow_conflicts_in_at_patterns(cx: &MatchVisitor<'_, '_>, pat: &Pat) { + // Get the mutability of `p` if it's by-ref. + let extract_binding_mut = |hir_id, span| match cx.tables.pat_binding_modes().get(hir_id) { + None => { + cx.tcx.sess.delay_span_bug(span, "missing binding mode"); + None + } + Some(ty::BindByValue(..)) => None, + Some(ty::BindByReference(m)) => Some(*m), + }; + pat.walk(|pat| { + // Extract `sub` in `binding @ sub`. + let (name, sub) = match &pat.kind { + hir::PatKind::Binding(.., name, Some(sub)) => (*name, sub), + _ => return true, + }; + + // Extract the mutability. + let mut_outer = match extract_binding_mut(pat.hir_id, pat.span) { + None => return true, + Some(m) => m, + }; + + // We now have `ref $mut_outer binding @ sub` (semantically). + // Recurse into each binding in `sub` and find mutability conflicts. + let mut conflicts_mut_mut = Vec::new(); + let mut conflicts_mut_ref = Vec::new(); + sub.each_binding(|_, hir_id, span, _| { + if let Some(mut_inner) = extract_binding_mut(hir_id, span) { + match (mut_outer, mut_inner) { + (Mutability::Immutable, Mutability::Immutable) => {} + (Mutability::Mutable, Mutability::Mutable) => conflicts_mut_mut.push(span), + _ => conflicts_mut_ref.push(span), + } + } + }); + + // Report errors if any. + let binding_span = pat.span.with_hi(name.span.hi()); + if !conflicts_mut_mut.is_empty() { + // Report mutability conflicts for e.g. `ref mut x @ Some(ref mut y)`. + let msg = &format!("cannot borrow `{}` as mutable more than once at a time", name); + let mut err = cx.tcx.sess.struct_span_err(pat.span, msg); + err.span_label(binding_span, "first mutable borrow occurs here"); + for sp in conflicts_mut_mut { + err.span_label(sp, "another mutable borrow occurs here"); + } + for sp in conflicts_mut_ref { + err.span_label(sp, "also borrowed as immutable here"); + } + err.emit(); + } else if !conflicts_mut_ref.is_empty() { + // Report mutability conflicts for e.g. `ref x @ Some(ref mut y)` or the converse. + let (primary, also) = match mut_outer { + Mutability::Mutable => ("mutable", "immutable"), + Mutability::Immutable => ("immutable", "mutable"), + }; + let msg = &format!( + "cannot borrow `{}` as {} because it is also borrowed as {}", + name, primary, also, + ); + let mut err = cx.tcx.sess.struct_span_err(pat.span, msg); + err.span_label(binding_span, &format!("{} borrow occurs here", primary)); + for sp in conflicts_mut_ref { + err.span_label(sp, &format!("{} borrow occurs here", also)); + } + err.emit(); + } -struct AtBindingPatternVisitor<'a, 'b, 'tcx> { - cx: &'a MatchVisitor<'b, 'tcx>, - bindings_allowed: bool, + true + }); } -impl<'v> Visitor<'v> for AtBindingPatternVisitor<'_, '_, '_> { - fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'v> { - NestedVisitorMap::None +/// Forbids bindings in `@` patterns. This used to be is necessary for memory safety, +/// because of the way rvalues were handled in the borrow check. (See issue #14587.) +fn check_legality_of_bindings_in_at_patterns(cx: &MatchVisitor<'_, '_>, pat: &Pat) { + AtBindingPatternVisitor { cx, bindings_allowed: true }.visit_pat(pat); + + struct AtBindingPatternVisitor<'a, 'b, 'tcx> { + cx: &'a MatchVisitor<'b, 'tcx>, + bindings_allowed: bool, } - fn visit_pat(&mut self, pat: &Pat) { - match pat.kind { - hir::PatKind::Binding(.., ref subpat) => { - if !self.bindings_allowed { - feature_err( - &self.cx.tcx.sess.parse_sess, - sym::bindings_after_at, - pat.span, - "pattern bindings after an `@` are unstable", - ) - .emit(); - } + impl<'v> Visitor<'v> for AtBindingPatternVisitor<'_, '_, '_> { + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'v> { + NestedVisitorMap::None + } - if subpat.is_some() { - let bindings_were_allowed = self.bindings_allowed; - self.bindings_allowed = false; - intravisit::walk_pat(self, pat); - self.bindings_allowed = bindings_were_allowed; + fn visit_pat(&mut self, pat: &Pat) { + match pat.kind { + hir::PatKind::Binding(.., ref subpat) => { + if !self.bindings_allowed { + feature_err( + &self.cx.tcx.sess.parse_sess, + sym::bindings_after_at, + pat.span, + "pattern bindings after an `@` are unstable", + ) + .emit(); + } + + if subpat.is_some() { + let bindings_were_allowed = self.bindings_allowed; + self.bindings_allowed = false; + intravisit::walk_pat(self, pat); + self.bindings_allowed = bindings_were_allowed; + } } + _ => intravisit::walk_pat(self, pat), } - _ => intravisit::walk_pat(self, pat), } } } -- 2.44.0