From aa6fb6caf9d8456c70144ccba1e969c85926e229 Mon Sep 17 00:00:00 2001 From: Matthew Jasper <mjjasper1@gmail.com> Date: Sat, 26 Jan 2019 17:25:37 +0000 Subject: [PATCH] Enable migrate mode by default on the 2015 edition This also fully stabilizes two-phase borrows on all editions --- src/librustc/infer/mod.rs | 4 +- src/librustc/session/config.rs | 17 +-- src/librustc/ty/context.rs | 60 +------- src/librustc_borrowck/borrowck/mod.rs | 6 - src/librustc_borrowck/borrowck/unused.rs | 116 ---------------- src/librustc_mir/borrow_check/borrow_set.rs | 5 +- src/librustc_mir/borrow_check/mod.rs | 55 +++----- .../borrow_check/nll/invalidation.rs | 4 - .../borrow_check/nll/type_check/mod.rs | 5 +- src/librustc_mir/borrow_check/path_utils.rs | 5 +- src/librustc_mir/build/expr/as_place.rs | 6 +- src/librustc_mir/build/matches/mod.rs | 86 +++++------- src/librustc_mir/error_codes.rs | 130 ++++++++++-------- src/librustc_mir/hair/pattern/check_match.rs | 6 +- src/librustc_mir/util/borrowck_errors.rs | 2 +- src/libsyntax/feature_gate.rs | 4 +- 16 files changed, 154 insertions(+), 357 deletions(-) delete mode 100644 src/librustc_borrowck/borrowck/unused.rs diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index 16140d006bf..747f0a6ae87 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -92,8 +92,8 @@ impl SuppressRegionErrors { /// enabled. pub fn when_nll_is_enabled(tcx: TyCtxt<'_, '_, '_>) -> Self { match tcx.borrowck_mode() { - // If we're on AST or Migrate mode, report AST region errors - BorrowckMode::Ast | BorrowckMode::Migrate => SuppressRegionErrors { suppressed: false }, + // If we're on Migrate mode, report AST region errors + BorrowckMode::Migrate => SuppressRegionErrors { suppressed: false }, // If we're on MIR or Compare mode, don't report AST region errors as they should // be reported by NLL diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 97a1c83dbff..cb307800fcd 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -460,7 +460,6 @@ pub enum PrintRequest { #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum BorrowckMode { - Ast, Mir, Compare, Migrate, @@ -471,7 +470,6 @@ impl BorrowckMode { /// on the AST borrow check if the MIR-based one errors. pub fn migrate(self) -> bool { match self { - BorrowckMode::Ast => false, BorrowckMode::Compare => false, BorrowckMode::Mir => false, BorrowckMode::Migrate => true, @@ -481,21 +479,11 @@ impl BorrowckMode { /// Should we emit the AST-based borrow checker errors? pub fn use_ast(self) -> bool { match self { - BorrowckMode::Ast => true, BorrowckMode::Compare => true, BorrowckMode::Mir => false, BorrowckMode::Migrate => false, } } - /// Should we emit the MIR-based borrow checker errors? - pub fn use_mir(self) -> bool { - match self { - BorrowckMode::Ast => false, - BorrowckMode::Compare => true, - BorrowckMode::Mir => true, - BorrowckMode::Migrate => true, - } - } } pub enum Input { @@ -627,7 +615,7 @@ impl Default for Options { incremental: None, debugging_opts: basic_debugging_options(), prints: Vec::new(), - borrowck_mode: BorrowckMode::Ast, + borrowck_mode: BorrowckMode::Migrate, cg: basic_codegen_options(), error_format: ErrorOutputType::default(), externs: Externs(BTreeMap::new()), @@ -2326,10 +2314,9 @@ pub fn build_session_options_and_crate_config( })); let borrowck_mode = match debugging_opts.borrowck.as_ref().map(|s| &s[..]) { - None | Some("ast") => BorrowckMode::Ast, + None | Some("migrate") => BorrowckMode::Migrate, Some("mir") => BorrowckMode::Mir, Some("compare") => BorrowckMode::Compare, - Some("migrate") => BorrowckMode::Migrate, Some(m) => early_error(error_format, &format!("unknown borrowck mode `{}`", m)), }; diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 1ce9ffd9433..ed500d1ac33 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -70,7 +70,6 @@ use rustc_macros::HashStable; use syntax::ast; use syntax::attr; use syntax::source_map::MultiSpan; -use syntax::edition::Edition; use syntax::feature_gate; use syntax::symbol::{Symbol, keywords, InternedString}; use syntax_pos::Span; @@ -1496,21 +1495,13 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { /// because that method has a narrower effect that can be toggled /// off via a separate `-Z` flag, at least for the short term. pub fn allow_bind_by_move_patterns_with_guards(self) -> bool { - self.features().bind_by_move_pattern_guards && self.use_mir_borrowck() + self.features().bind_by_move_pattern_guards } /// If true, we should use a naive AST walk to determine if match /// guard could perform bad mutations (or mutable-borrows). pub fn check_for_mutation_in_guard_via_ast_walk(self) -> bool { - // If someone requests the feature, then be a little more - // careful and ensure that MIR-borrowck is enabled (which can - // happen via edition selection, via `feature(nll)`, or via an - // appropriate `-Z` flag) before disabling the mutation check. - if self.allow_bind_by_move_patterns_with_guards() { - return false; - } - - return true; + !self.allow_bind_by_move_patterns_with_guards() } /// If true, we should use the AST-based borrowck (we may *also* use @@ -1519,12 +1510,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { self.borrowck_mode().use_ast() } - /// If true, we should use the MIR-based borrowck (we may *also* use - /// the AST-based borrowck). - pub fn use_mir_borrowck(self) -> bool { - self.borrowck_mode().use_mir() - } - /// If true, we should use the MIR-based borrow check, but also /// fall back on the AST borrow check if the MIR-based one errors. pub fn migrate_borrowck(self) -> bool { @@ -1541,23 +1526,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { /// statements (which simulate the maximal effect of executing the /// patterns in a match arm). pub fn emit_read_for_match(&self) -> bool { - self.use_mir_borrowck() && !self.sess.opts.debugging_opts.nll_dont_emit_read_for_match - } - - /// If true, pattern variables for use in guards on match arms - /// will be bound as references to the data, and occurrences of - /// those variables in the guard expression will implicitly - /// dereference those bindings. (See rust-lang/rust#27282.) - pub fn all_pat_vars_are_implicit_refs_within_guards(self) -> bool { - self.borrowck_mode().use_mir() - } - - /// If true, we should enable two-phase borrows checks. This is - /// done with either: `-Ztwo-phase-borrows`, `#![feature(nll)]`, - /// or by opting into an edition after 2015. - pub fn two_phase_borrows(self) -> bool { - self.sess.rust_2018() || self.features().nll || - self.sess.opts.debugging_opts.two_phase_borrows + !self.sess.opts.debugging_opts.nll_dont_emit_read_for_match } /// What mode(s) of borrowck should we run? AST? MIR? both? @@ -1565,14 +1534,10 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { pub fn borrowck_mode(&self) -> BorrowckMode { // Here are the main constraints we need to deal with: // - // 1. An opts.borrowck_mode of `BorrowckMode::Ast` is + // 1. An opts.borrowck_mode of `BorrowckMode::Migrate` is // synonymous with no `-Z borrowck=...` flag at all. - // (This is arguably a historical accident.) // - // 2. `BorrowckMode::Migrate` is the limited migration to - // NLL that we are deploying with the 2018 edition. - // - // 3. We want to allow developers on the Nightly channel + // 2. We want to allow developers on the Nightly channel // to opt back into the "hard error" mode for NLL, // (which they can do via specifying `#![feature(nll)]` // explicitly in their crate). @@ -1585,24 +1550,13 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { // a user's attempt to specify `-Z borrowck=compare`, which // we arguably do not need anymore and should remove.) // - // * Otherwise, if no `-Z borrowck=...` flag was given (or - // if `borrowck=ast` was specified), then use the default - // as required by the edition. + // * Otherwise, if no `-Z borrowck=...` then use migrate mode // // * Otherwise, use the behavior requested via `-Z borrowck=...` if self.features().nll { return BorrowckMode::Mir; } - match self.sess.opts.borrowck_mode { - mode @ BorrowckMode::Mir | - mode @ BorrowckMode::Compare | - mode @ BorrowckMode::Migrate => mode, - - BorrowckMode::Ast => match self.sess.edition() { - Edition::Edition2015 => BorrowckMode::Ast, - Edition::Edition2018 => BorrowckMode::Migrate, - }, - } + self.sess.opts.borrowck_mode } #[inline] diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index a2f6d9713f0..540f374bcf9 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -49,8 +49,6 @@ pub mod gather_loans; pub mod move_data; -mod unused; - #[derive(Clone, Copy)] pub struct LoanDataFlowOperator; @@ -138,10 +136,6 @@ fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId) check_loans::check_loans(&mut bccx, &loan_dfcx, &flowed_moves, &all_loans, body); } - if !tcx.use_mir_borrowck() { - unused::check(&mut bccx, body); - } - Lrc::new(BorrowCheckResult { used_mut_nodes: bccx.used_mut_nodes.into_inner(), signalled_any_error: bccx.signalled_any_error.into_inner(), diff --git a/src/librustc_borrowck/borrowck/unused.rs b/src/librustc_borrowck/borrowck/unused.rs deleted file mode 100644 index 60a9c18e95e..00000000000 --- a/src/librustc_borrowck/borrowck/unused.rs +++ /dev/null @@ -1,116 +0,0 @@ -use rustc::hir::intravisit::{Visitor, NestedVisitorMap}; -use rustc::hir::{self, HirId}; -use rustc::lint::builtin::UNUSED_MUT; -use rustc::ty; -use rustc::util::nodemap::{FxHashMap, FxHashSet}; -use errors::Applicability; -use std::slice; -use syntax::ptr::P; - -use crate::borrowck::BorrowckCtxt; - -pub fn check<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, body: &'tcx hir::Body) { - let mut used_mut = bccx.used_mut_nodes.borrow().clone(); - UsedMutFinder { - bccx, - set: &mut used_mut, - }.visit_expr(&body.value); - let mut cx = UnusedMutCx { bccx, used_mut }; - for arg in body.arguments.iter() { - cx.check_unused_mut_pat(slice::from_ref(&arg.pat)); - } - cx.visit_expr(&body.value); -} - -struct UsedMutFinder<'a, 'tcx: 'a> { - bccx: &'a BorrowckCtxt<'a, 'tcx>, - set: &'a mut FxHashSet<HirId>, -} - -struct UnusedMutCx<'a, 'tcx: 'a> { - bccx: &'a BorrowckCtxt<'a, 'tcx>, - used_mut: FxHashSet<HirId>, -} - -impl<'a, 'tcx> UnusedMutCx<'a, 'tcx> { - fn check_unused_mut_pat(&self, pats: &[P<hir::Pat>]) { - let tcx = self.bccx.tcx; - let mut mutables: FxHashMap<_, Vec<_>> = Default::default(); - for p in pats { - p.each_binding(|_, hir_id, span, ident| { - // Skip anything that looks like `_foo` - if ident.as_str().starts_with("_") { - return; - } - - // Skip anything that looks like `&foo` or `&mut foo`, only look - // for by-value bindings - if let Some(&bm) = self.bccx.tables.pat_binding_modes().get(hir_id) { - match bm { - ty::BindByValue(hir::MutMutable) => {} - _ => return, - } - - mutables.entry(ident.name).or_default().push((hir_id, span)); - } else { - tcx.sess.delay_span_bug(span, "missing binding mode"); - } - }); - } - - for (_name, ids) in mutables { - // If any id for this name was used mutably then consider them all - // ok, so move on to the next - if ids.iter().any(|&(ref hir_id, _)| self.used_mut.contains(hir_id)) { - continue; - } - - let (hir_id, span) = ids[0]; - if span.compiler_desugaring_kind().is_some() { - // If the `mut` arises as part of a desugaring, we should ignore it. - continue; - } - - // Ok, every name wasn't used mutably, so issue a warning that this - // didn't need to be mutable. - let mut_span = tcx.sess.source_map().span_until_non_whitespace(span); - tcx.struct_span_lint_hir(UNUSED_MUT, - hir_id, - span, - "variable does not need to be mutable") - .span_suggestion_short( - mut_span, - "remove this `mut`", - String::new(), - Applicability::MachineApplicable, - ) - .emit(); - } - } -} - -impl<'a, 'tcx> Visitor<'tcx> for UnusedMutCx<'a, 'tcx> { - fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { - NestedVisitorMap::OnlyBodies(&self.bccx.tcx.hir()) - } - - fn visit_arm(&mut self, arm: &hir::Arm) { - self.check_unused_mut_pat(&arm.pats) - } - - fn visit_local(&mut self, local: &hir::Local) { - self.check_unused_mut_pat(slice::from_ref(&local.pat)); - } -} - -impl<'a, 'tcx> Visitor<'tcx> for UsedMutFinder<'a, 'tcx> { - fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { - NestedVisitorMap::OnlyBodies(&self.bccx.tcx.hir()) - } - - fn visit_nested_body(&mut self, id: hir::BodyId) { - let def_id = self.bccx.tcx.hir().body_owner_def_id(id); - self.set.extend(self.bccx.tcx.borrowck(def_id).used_mut_nodes.iter().cloned()); - self.visit_body(self.bccx.tcx.hir().body(id)); - } -} diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index c81da66672f..918192395c3 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -303,9 +303,8 @@ impl<'a, 'gcx, 'tcx> GatherBorrows<'a, 'gcx, 'tcx> { /// allowed to be split into separate Reservation and /// Activation phases. fn allow_two_phase_borrow(&self, kind: mir::BorrowKind) -> bool { - self.tcx.two_phase_borrows() - && (kind.allows_two_phase_borrow() - || self.tcx.sess.opts.debugging_opts.two_phase_beyond_autoref) + kind.allows_two_phase_borrow() + || self.tcx.sess.opts.debugging_opts.two_phase_beyond_autoref } /// If this is a two-phase borrow, then we will record it diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 43ed85d4ac5..4a3159d8a9d 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -74,37 +74,28 @@ fn mir_borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> BorrowC let input_mir = tcx.mir_validated(def_id); debug!("run query mir_borrowck: {}", tcx.def_path_str(def_id)); - let mut return_early; - - // Return early if we are not supposed to use MIR borrow checker for this function. - return_early = !tcx.has_attr(def_id, "rustc_mir") && !tcx.use_mir_borrowck(); - + // We are not borrow checking the automatically generated struct/variant constructors + // because we want to accept structs such as this (taken from the `linked-hash-map` + // crate): + // ```rust + // struct Qey<Q: ?Sized>(Q); + // ``` + // MIR of this struct constructor looks something like this: + // ```rust + // fn Qey(_1: Q) -> Qey<Q>{ + // let mut _0: Qey<Q>; // return place + // + // bb0: { + // (_0.0: Q) = move _1; // bb0[0]: scope 0 at src/main.rs:1:1: 1:26 + // return; // bb0[1]: scope 0 at src/main.rs:1:1: 1:26 + // } + // } + // ``` + // The problem here is that `(_0.0: Q) = move _1;` is valid only if `Q` is + // of statically known size, which is not known to be true because of the + // `Q: ?Sized` constraint. However, it is true because the constructor can be + // called only when `Q` is of statically known size. if tcx.is_constructor(def_id) { - // We are not borrow checking the automatically generated struct/variant constructors - // because we want to accept structs such as this (taken from the `linked-hash-map` - // crate): - // ```rust - // struct Qey<Q: ?Sized>(Q); - // ``` - // MIR of this struct constructor looks something like this: - // ```rust - // fn Qey(_1: Q) -> Qey<Q>{ - // let mut _0: Qey<Q>; // return place - // - // bb0: { - // (_0.0: Q) = move _1; // bb0[0]: scope 0 at src/main.rs:1:1: 1:26 - // return; // bb0[1]: scope 0 at src/main.rs:1:1: 1:26 - // } - // } - // ``` - // The problem here is that `(_0.0: Q) = move _1;` is valid only if `Q` is - // of statically known size, which is not known to be true because of the - // `Q: ?Sized` constraint. However, it is true because the constructor can be - // called only when `Q` is of statically known size. - return_early = true; - } - - if return_early { return BorrowCheckResult { closure_requirements: None, used_mut_upvars: SmallVec::new(), @@ -1505,10 +1496,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { span: Span, flow_state: &Flows<'cx, 'gcx, 'tcx>, ) { - if !self.infcx.tcx.two_phase_borrows() { - return; - } - // Two-phase borrow support: For each activation that is newly // generated at this statement, check if it interferes with // another borrow. diff --git a/src/librustc_mir/borrow_check/nll/invalidation.rs b/src/librustc_mir/borrow_check/nll/invalidation.rs index 36d3a03cdfd..a5230e67b75 100644 --- a/src/librustc_mir/borrow_check/nll/invalidation.rs +++ b/src/librustc_mir/borrow_check/nll/invalidation.rs @@ -474,10 +474,6 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> { &mut self, location: Location, ) { - if !self.tcx.two_phase_borrows() { - return; - } - // Two-phase borrow support: For each activation that is newly // generated at this statement, check if it interferes with // another borrow. diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index ce8f1852551..0dee64db727 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -2672,9 +2672,8 @@ impl MirPass for TypeckMir { let def_id = src.def_id(); debug!("run_pass: {:?}", def_id); - // When NLL is enabled, the borrow checker runs the typeck - // itself, so we don't need this MIR pass anymore. - if tcx.use_mir_borrowck() { + // FIXME: We don't need this MIR pass anymore. + if true { return; } diff --git a/src/librustc_mir/borrow_check/path_utils.rs b/src/librustc_mir/borrow_check/path_utils.rs index 42eb502b907..c68dee29c5b 100644 --- a/src/librustc_mir/borrow_check/path_utils.rs +++ b/src/librustc_mir/borrow_check/path_utils.rs @@ -15,9 +15,8 @@ pub(super) fn allow_two_phase_borrow<'a, 'tcx, 'gcx: 'tcx>( tcx: &TyCtxt<'a, 'gcx, 'tcx>, kind: BorrowKind ) -> bool { - tcx.two_phase_borrows() - && (kind.allows_two_phase_borrow() - || tcx.sess.opts.debugging_opts.two_phase_beyond_autoref) + kind.allows_two_phase_borrow() + || tcx.sess.opts.debugging_opts.two_phase_beyond_autoref } /// Control for the path borrow checking code diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index b500060684f..713e3fe0218 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -112,11 +112,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } ExprKind::SelfRef => block.and(Place::Base(PlaceBase::Local(Local::new(1)))), ExprKind::VarRef { id } => { - let place = if this.is_bound_var_in_guard(id) && this - .hir - .tcx() - .all_pat_vars_are_implicit_refs_within_guards() - { + let place = if this.is_bound_var_in_guard(id) { let index = this.var_local_id(id, RefWithinGuard); Place::Base(PlaceBase::Local(index)).deref() } else { diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index 566f1790f8f..b5b2d78f1bd 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -1425,26 +1425,22 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // the reference that we create for the arm. // * So we eagerly create the reference for the arm and then take a // reference to that. - let tcx = self.hir.tcx(); - let autoref = tcx.all_pat_vars_are_implicit_refs_within_guards(); if let Some(guard) = guard { - if autoref { - self.bind_matched_candidate_for_guard( - block, - &candidate.bindings, - ); - let guard_frame = GuardFrame { - locals: candidate - .bindings - .iter() - .map(|b| GuardFrameLocal::new(b.var_id, b.binding_mode)) - .collect(), - }; - debug!("Entering guard building context: {:?}", guard_frame); - self.guard_context.push(guard_frame); - } else { - self.bind_matched_candidate_for_arm_body(block, &candidate.bindings); - } + let tcx = self.hir.tcx(); + + self.bind_matched_candidate_for_guard( + block, + &candidate.bindings, + ); + let guard_frame = GuardFrame { + locals: candidate + .bindings + .iter() + .map(|b| GuardFrameLocal::new(b.var_id, b.binding_mode)) + .collect(), + }; + debug!("Entering guard building context: {:?}", guard_frame); + self.guard_context.push(guard_frame); let re_erased = tcx.types.re_erased; let scrutinee_source_info = self.source_info(scrutinee_span); @@ -1470,13 +1466,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let source_info = self.source_info(guard.span); let guard_end = self.source_info(tcx.sess.source_map().end_point(guard.span)); let cond = unpack!(block = self.as_local_operand(block, guard)); - if autoref { - let guard_frame = self.guard_context.pop().unwrap(); - debug!( - "Exiting guard building context with locals: {:?}", - guard_frame - ); - } + let guard_frame = self.guard_context.pop().unwrap(); + debug!( + "Exiting guard building context with locals: {:?}", + guard_frame + ); for &(_, temp) in fake_borrows { self.cfg.push(block, Statement { @@ -1526,28 +1520,26 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ), ); - if autoref { - let by_value_bindings = candidate.bindings.iter().filter(|binding| { - if let BindingMode::ByValue = binding.binding_mode { true } else { false } - }); - // Read all of the by reference bindings to ensure that the - // place they refer to can't be modified by the guard. - for binding in by_value_bindings.clone() { - let local_id = self.var_local_id(binding.var_id, RefWithinGuard); + let by_value_bindings = candidate.bindings.iter().filter(|binding| { + if let BindingMode::ByValue = binding.binding_mode { true } else { false } + }); + // Read all of the by reference bindings to ensure that the + // place they refer to can't be modified by the guard. + for binding in by_value_bindings.clone() { + let local_id = self.var_local_id(binding.var_id, RefWithinGuard); let place = Place::Base(PlaceBase::Local(local_id)); - self.cfg.push( - block, - Statement { - source_info: guard_end, - kind: StatementKind::FakeRead(FakeReadCause::ForGuardBinding, place), - }, - ); - } - self.bind_matched_candidate_for_arm_body( - post_guard_block, - by_value_bindings, + self.cfg.push( + block, + Statement { + source_info: guard_end, + kind: StatementKind::FakeRead(FakeReadCause::ForGuardBinding, place), + }, ); } + self.bind_matched_candidate_for_arm_body( + post_guard_block, + by_value_bindings, + ); self.cfg.terminate( post_guard_block, @@ -1604,8 +1596,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } - // Only called when all_pat_vars_are_implicit_refs_within_guards, - // and thus all code/comments assume we are in that context. fn bind_matched_candidate_for_guard( &mut self, block: BasicBlock, @@ -1739,7 +1729,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }))), }; let for_arm_body = self.local_decls.push(local.clone()); - let locals = if has_guard.0 && tcx.all_pat_vars_are_implicit_refs_within_guards() { + let locals = if has_guard.0 { let ref_for_guard = self.local_decls.push(LocalDecl::<'tcx> { // This variable isn't mutated but has a name, so has to be // immutable to avoid the unused mut lint. diff --git a/src/librustc_mir/error_codes.rs b/src/librustc_mir/error_codes.rs index c8836fe5193..74a4278d599 100644 --- a/src/librustc_mir/error_codes.rs +++ b/src/librustc_mir/error_codes.rs @@ -877,12 +877,14 @@ https://doc.rust-lang.org/book/ch04-00-understanding-ownership.html "##, E0383: r##" +#### Note: this error code is no longer emitted by the compiler. + This error occurs when an attempt is made to partially reinitialize a structure that is currently uninitialized. For example, this can happen when a drop has taken place: -```compile_fail,E0383 +```compile_fail struct Foo { a: u32, } @@ -966,10 +968,12 @@ y.set(2); "##,*/ E0387: r##" +#### Note: this error code is no longer emitted by the compiler. + This error occurs when an attempt is made to mutate or mutably reference data that a closure has captured immutably. Examples of this error are shown below: -```compile_fail,E0387 +```compile_fail // Accepts a function or a closure that captures its environment immutably. // Closures passed to foo will not be able to mutate their closed-over state. fn foo<F: Fn()>(f: F) { } @@ -1026,13 +1030,15 @@ E0388 was removed and is no longer issued. "##, E0389: r##" +#### Note: this error code is no longer emitted by the compiler. + An attempt was made to mutate data using a non-mutable reference. This commonly occurs when attempting to assign to a non-mutable reference of a mutable reference (`&(&mut T)`). Example of erroneous code: -```compile_fail,E0389 +```compile_fail struct FancyNum { num: u8, } @@ -1202,6 +1208,7 @@ A variable was borrowed as mutable more than once. Erroneous code example: let mut i = 0; let mut x = &mut i; let mut a = &mut i; +x; // error: cannot borrow `i` as mutable more than once at a time ``` @@ -1220,35 +1227,33 @@ let mut i = 0; let a = &i; // ok! let b = &i; // still ok! let c = &i; // still ok! +b; +a; ``` "##, E0500: r##" -A borrowed variable was used in another closure. Example of erroneous code: +A borrowed variable was used by a closure. Example of erroneous code: -```compile_fail +```compile_fail,E0500 fn you_know_nothing(jon_snow: &mut i32) { - let nights_watch = || { - *jon_snow = 2; - }; + let nights_watch = &jon_snow; let starks = || { *jon_snow = 3; // error: closure requires unique access to `jon_snow` // but it is already borrowed }; + println!("{}", nights_watch); } ``` -In here, `jon_snow` is already borrowed by the `nights_watch` closure, so it +In here, `jon_snow` is already borrowed by the `nights_watch` reference, so it cannot be borrowed by the `starks` closure at the same time. To fix this issue, -you can put the closure in its own scope: +you can create the closure after the borrow has ended: ``` fn you_know_nothing(jon_snow: &mut i32) { - { - let nights_watch = || { - *jon_snow = 2; - }; - } // At this point, `jon_snow` is free. + let nights_watch = &jon_snow; + println!("{}", nights_watch); let starks = || { *jon_snow = 3; }; @@ -1261,12 +1266,10 @@ closures: ``` fn you_know_nothing(jon_snow: &mut i32) { let mut jon_copy = jon_snow.clone(); - let nights_watch = || { - jon_copy = 2; - }; let starks = || { *jon_snow = 3; }; + println!("{}", jon_copy); } ``` "##, @@ -1293,26 +1296,28 @@ fn outside_closure(x: &mut i32) { } fn foo(a: &mut i32) { - let bar = || { + let mut bar = || { inside_closure(a) }; outside_closure(a); // error: cannot borrow `*a` as mutable because previous // closure requires unique access. + bar(); } ``` -To fix this error, you can place the closure in its own scope: +To fix this error, you can finish using the closure before using the captured +variable: ``` fn inside_closure(x: &mut i32) {} fn outside_closure(x: &mut i32) {} fn foo(a: &mut i32) { - { - let bar = || { - inside_closure(a) - }; - } // borrow on `a` ends. + let mut bar = || { + inside_closure(a) + }; + bar(); + // borrow on `a` ends. outside_closure(a); // ok! } ``` @@ -1324,7 +1329,7 @@ fn inside_closure(x: &mut i32) {} fn outside_closure(x: &mut i32) {} fn foo(a: &mut i32) { - let bar = |s: &mut i32| { + let mut bar = |s: &mut i32| { inside_closure(s) }; outside_closure(a); @@ -1340,9 +1345,10 @@ fn outside_closure(x: &mut i32) {} fn foo(a: &mut i32) { outside_closure(a); - let bar = || { + let mut bar = || { inside_closure(a) }; + bar(); } ``` "##, @@ -1359,6 +1365,7 @@ fn foo(a: &mut i32) { let ref y = a; // a is borrowed as immutable. bar(a); // error: cannot borrow `*a` as mutable because `a` is also borrowed // as immutable + println!("{}", y); } ``` @@ -1370,6 +1377,7 @@ fn bar(x: &mut i32) {} fn foo(a: &mut i32) { bar(a); let ref y = a; // ok! + println!("{}", y); } ``` @@ -1385,11 +1393,11 @@ Example of erroneous code: ```compile_fail,E0503 fn main() { let mut value = 3; - // Create a mutable borrow of `value`. This borrow - // lives until the end of this function. - let _borrow = &mut value; + // Create a mutable borrow of `value`. + let borrow = &mut value; let _sum = value + 1; // error: cannot use `value` because // it was mutably borrowed + println!("{}", borrow); } ``` @@ -1397,16 +1405,14 @@ In this example, `value` is mutably borrowed by `borrow` and cannot be used to calculate `sum`. This is not possible because this would violate Rust's mutability rules. -You can fix this error by limiting the scope of the borrow: +You can fix this error by finishing using the borrow before the next use of +the value: ``` fn main() { let mut value = 3; - // By creating a new block, you can limit the scope - // of the reference. - { - let _borrow = &mut value; // Use `_borrow` inside this block. - } + let borrow = &mut value; + println!("{}", borrow); // The block has ended and with it the borrow. // You can now use `value` again. let _sum = value + 1; @@ -1422,10 +1428,11 @@ fn main() { let value_cloned = value.clone(); // The mutable borrow is a reference to `value` and // not to `value_cloned`... - let _borrow = &mut value; + let borrow = &mut value; // ... which means we can still use `value_cloned`, let _sum = value_cloned + 1; // even though the borrow only ends here. + println!("{}", borrow); } ``` @@ -1434,12 +1441,14 @@ http://doc.rust-lang.org/book/ch04-02-references-and-borrowing.html "##, E0504: r##" +#### Note: this error code is no longer emitted by the compiler. + This error occurs when an attempt is made to move a borrowed variable into a closure. Example of erroneous code: -```compile_fail,E0504 +```compile_fail struct FancyNum { num: u8, } @@ -1577,9 +1586,10 @@ fn eat(val: &Value) {} fn main() { let x = Value{}; - let _ref_to_val: &Value = &x; + + let ref_to_val: &Value = &x; eat(&x); // pass by reference, if it's possible - borrow(_ref_to_val); + borrow(ref_to_val); } ``` @@ -1594,11 +1604,11 @@ fn eat(val: Value) {} fn main() { let x = Value{}; - { - let _ref_to_val: &Value = &x; - borrow(_ref_to_val); - } - eat(x); // release borrow and then move it. + + let ref_to_val: &Value = &x; + borrow(ref_to_val); + // ref_to_val is no longer used. + eat(x); } ``` @@ -1614,9 +1624,9 @@ fn eat(val: Value) {} fn main() { let x = Value{}; - let _ref_to_val: &Value = &x; + let ref_to_val: &Value = &x; eat(x); // it will be copied here. - borrow(_ref_to_val); + borrow(ref_to_val); } ``` @@ -2053,11 +2063,13 @@ fn get_owned_iterator() -> IntoIter<i32> { "##, E0595: r##" +#### Note: this error code is no longer emitted by the compiler. + Closures cannot mutate immutable captured variables. Erroneous code example: -```compile_fail,E0595 +```compile_fail,E0594 let x = 3; // error: closure cannot assign to immutable local variable `x` let mut c = || { x += 1 }; ``` @@ -2090,8 +2102,7 @@ let y = &mut x; // ok! "##, E0597: r##" -This error occurs because a borrow was made inside a variable which has a -greater lifetime than the borrowed one. +This error occurs because a value was dropped while it was still borrowed Example of erroneous code: @@ -2101,23 +2112,28 @@ struct Foo<'a> { } let mut x = Foo { x: None }; -let y = 0; -x.x = Some(&y); // error: `y` does not live long enough +{ + let y = 0; + x.x = Some(&y); // error: `y` does not live long enough +} +println!("{:?}", x.x); ``` -In here, `x` is created before `y` and therefore has a greater lifetime. Always -keep in mind that values in a scope are dropped in the opposite order they are -created. So to fix the previous example, just make the `y` lifetime greater than -the `x`'s one: +In here, `y` is dropped at the end of the inner scope, but it is borrowed by +`x` until the `println`. To fix the previous example, just remove the scope +so that `y` isn't dropped until after the println ``` struct Foo<'a> { x: Option<&'a u32>, } -let y = 0; let mut x = Foo { x: None }; + +let y = 0; x.x = Some(&y); + +println!("{:?}", x.x); ``` "##, diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index 7ded973701e..7bfb0a4475e 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -565,7 +565,7 @@ fn check_legality_of_move_bindings( let mut err = struct_span_err!(cx.tcx.sess, p.span, E0008, "cannot bind by-move into a pattern guard"); err.span_label(p.span, "moves value into pattern guard"); - if cx.tcx.sess.opts.unstable_features.is_nightly_build() && cx.tcx.use_mir_borrowck() { + if cx.tcx.sess.opts.unstable_features.is_nightly_build() { err.help("add #![feature(bind_by_move_pattern_guards)] to the \ crate attributes to enable"); } @@ -649,9 +649,7 @@ impl<'a, 'tcx> Delegate<'tcx> for MutationChecker<'a, 'tcx> { let mut err = struct_span_err!(self.cx.tcx.sess, span, E0301, "cannot mutably borrow in a pattern guard"); err.span_label(span, "borrowed mutably in pattern guard"); - if self.cx.tcx.sess.opts.unstable_features.is_nightly_build() && - self.cx.tcx.use_mir_borrowck() - { + if self.cx.tcx.sess.opts.unstable_features.is_nightly_build() { err.help("add #![feature(bind_by_move_pattern_guards)] to the \ crate attributes to enable"); } diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index fd694ddbbd1..e334e27cc85 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -40,7 +40,7 @@ impl Origin { pub fn should_emit_errors(self, mode: BorrowckMode) -> bool { match self { Origin::Ast => mode.use_ast(), - Origin::Mir => mode.use_mir(), + Origin::Mir => true, } } } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index bc87a88f9f1..ba567a123ae 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -445,9 +445,7 @@ declare_features! ( (active, custom_inner_attributes, "1.30.0", Some(54726), None), // Allow mixing of bind-by-move in patterns and references to - // those identifiers in guards, *if* we are using MIR-borrowck - // (aka NLL). Essentially this means you need to be using the - // 2018 edition or later. + // those identifiers in guards. (active, bind_by_move_pattern_guards, "1.30.0", Some(15287), None), // Allows `impl Trait` in bindings (`let`, `const`, `static`).