diff --git a/src/librustc_data_structures/bitvec.rs b/src/librustc_data_structures/bitvec.rs index 28e3180063c..a22dd1fecec 100644 --- a/src/librustc_data_structures/bitvec.rs +++ b/src/librustc_data_structures/bitvec.rs @@ -8,11 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::collections::BTreeMap; -use std::collections::btree_map::Entry; -use std::marker::PhantomData; -use std::iter::FromIterator; use indexed_vec::{Idx, IndexVec}; +use std::collections::btree_map::Entry; +use std::collections::BTreeMap; +use std::iter::FromIterator; +use std::marker::PhantomData; type Word = u128; const WORD_BITS: usize = 128; @@ -317,14 +317,25 @@ impl SparseBitMatrix { if read != write { let (bit_set_read, bit_set_write) = self.vector.pick2_mut(read, write); - for read_val in bit_set_read.iter() { - changed = changed | bit_set_write.insert(read_val); + for read_chunk in bit_set_read.chunks() { + changed = changed | bit_set_write.insert_chunk(read_chunk).any(); } } changed } + /// True if `sub` is a subset of `sup` + pub fn is_subset(&self, sub: R, sup: R) -> bool { + sub == sup || { + let bit_set_sub = &self.vector[sub]; + let bit_set_sup = &self.vector[sup]; + bit_set_sub + .chunks() + .all(|read_chunk| read_chunk.bits_eq(bit_set_sup.contains_chunk(read_chunk))) + } + } + /// Iterates through all the columns set to true in a given row of /// the matrix. pub fn iter<'a>(&'a self, row: R) -> impl Iterator + 'a { @@ -346,6 +357,7 @@ pub struct SparseChunk { } impl SparseChunk { + #[inline] pub fn one(index: I) -> Self { let index = index.index(); let key_usize = index / 128; @@ -358,10 +370,16 @@ impl SparseChunk { } } + #[inline] pub fn any(&self) -> bool { self.bits != 0 } + #[inline] + pub fn bits_eq(&self, other: SparseChunk) -> bool { + self.bits == other.bits + } + pub fn iter(&self) -> impl Iterator { let base = self.key as usize * 128; let mut bits = self.bits; @@ -394,6 +412,10 @@ impl SparseBitSet { self.chunk_bits.len() * 128 } + /// Returns a chunk containing only those bits that are already + /// present. You can test therefore if `self` contains all the + /// bits in chunk already by doing `chunk == + /// self.contains_chunk(chunk)`. pub fn contains_chunk(&self, chunk: SparseChunk) -> SparseChunk { SparseChunk { bits: self.chunk_bits @@ -403,6 +425,11 @@ impl SparseBitSet { } } + /// Modifies `self` to contain all the bits from `chunk` (in + /// addition to any pre-existing bits); returns a new chunk that + /// contains only those bits that were newly added. You can test + /// if anything was inserted by invoking `any()` on the returned + /// value. pub fn insert_chunk(&mut self, chunk: SparseChunk) -> SparseChunk { if chunk.bits == 0 { return chunk; diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 1cc69351b47..3d39042e9f4 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -10,7 +10,7 @@ //! This query borrow-checks the MIR to (further) ensure it is not broken. -use borrow_check::nll::region_infer::{RegionCausalInfo, RegionInferenceContext}; +use borrow_check::nll::region_infer::RegionInferenceContext; use rustc::hir; use rustc::hir::def_id::DefId; use rustc::hir::map::definitions::DefPathData; @@ -248,7 +248,6 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( nonlexical_regioncx: regioncx, used_mut: FxHashSet(), used_mut_upvars: SmallVec::new(), - nonlexical_cause_info: None, borrow_set, dominators, }; @@ -367,7 +366,6 @@ pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> { /// contains the results from region inference and lets us e.g. /// find out which CFG points are contained in each borrow region. nonlexical_regioncx: Rc>, - nonlexical_cause_info: Option, /// The set of borrows extracted from the MIR borrow_set: Rc>, diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs index 56e388a5b60..2807a4e8857 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -32,13 +32,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let regioncx = &&self.nonlexical_regioncx; let mir = self.mir; - if self.nonlexical_cause_info.is_none() { - self.nonlexical_cause_info = Some(regioncx.compute_causal_info(mir)); - } - - let cause_info = self.nonlexical_cause_info.as_ref().unwrap(); - if let Some(cause) = cause_info.why_region_contains_point(borrow.region, context.loc) { - match *cause.root_cause() { + let borrow_region_vid = regioncx.to_region_vid(borrow.region); + if let Some(cause) = regioncx.why_region_contains_point(borrow_region_vid, context.loc) { + match cause { Cause::LiveVar(local, location) => { match find_regular_use(mir, regioncx, borrow, location, local) { Some(p) => { diff --git a/src/librustc_mir/borrow_check/nll/region_infer/dfs.rs b/src/librustc_mir/borrow_check/nll/region_infer/dfs.rs deleted file mode 100644 index f68394d6149..00000000000 --- a/src/librustc_mir/borrow_check/nll/region_infer/dfs.rs +++ /dev/null @@ -1,265 +0,0 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -//! Module defining the `dfs` method on `RegionInferenceContext`, along with -//! its associated helper traits. - -use borrow_check::nll::universal_regions::UniversalRegions; -use borrow_check::nll::region_infer::RegionInferenceContext; -use borrow_check::nll::region_infer::values::{RegionElementIndex, RegionValueElements, - RegionValues}; -use syntax::codemap::Span; -use rustc::mir::{Location, Mir}; -use rustc::ty::RegionVid; -use rustc_data_structures::bitvec::BitVector; -use rustc_data_structures::indexed_vec::Idx; - -pub(super) struct DfsStorage { - stack: Vec, - visited: BitVector, -} - -impl<'tcx> RegionInferenceContext<'tcx> { - /// Creates dfs storage for use by dfs; this should be shared - /// across as many calls to dfs as possible to amortize allocation - /// costs. - pub(super) fn new_dfs_storage(&self) -> DfsStorage { - let num_elements = self.elements.num_elements(); - DfsStorage { - stack: vec![], - visited: BitVector::new(num_elements), - } - } - - /// Function used to satisfy or test a `R1: R2 @ P` - /// constraint. The core idea is that it performs a DFS starting - /// from `P`. The precise actions *during* that DFS depend on the - /// `op` supplied, so see (e.g.) `CopyFromSourceToTarget` for more - /// details. - /// - /// Returns: - /// - /// - `Ok(true)` if the walk was completed and something changed - /// along the way; - /// - `Ok(false)` if the walk was completed with no changes; - /// - `Err(early)` if the walk was existed early by `op`. `earlyelem` is the - /// value that `op` returned. - #[inline(never)] // ensure dfs is identifiable in profiles - pub(super) fn dfs( - &self, - mir: &Mir<'tcx>, - dfs: &mut DfsStorage, - mut op: C, - ) -> Result - where - C: DfsOp, - { - let mut changed = false; - - dfs.visited.clear(); - dfs.stack.push(op.start_point()); - while let Some(p) = dfs.stack.pop() { - let point_index = self.elements.index(p); - - if !op.source_region_contains(point_index) { - debug!(" not in from-region"); - continue; - } - - if !dfs.visited.insert(point_index.index()) { - debug!(" already visited"); - continue; - } - - let new = op.add_to_target_region(point_index)?; - changed |= new; - - let block_data = &mir[p.block]; - - let start_stack_len = dfs.stack.len(); - - if p.statement_index < block_data.statements.len() { - dfs.stack.push(Location { - statement_index: p.statement_index + 1, - ..p - }); - } else { - dfs.stack.extend( - block_data - .terminator() - .successors() - .map(|&basic_block| Location { - statement_index: 0, - block: basic_block, - }), - ); - } - - if dfs.stack.len() == start_stack_len { - // If we reach the END point in the graph, then copy - // over any skolemized end points in the `from_region` - // and make sure they are included in the `to_region`. - changed |= op.add_universal_regions_outlived_by_source_to_target()?; - } - } - - Ok(changed) - } -} - -/// Customizes the operation of the `dfs` function. This function is -/// used during inference to satisfy a `R1: R2 @ P` constraint. -pub(super) trait DfsOp { - /// If this op stops the walk early, what type does it propagate? - type Early; - - /// Returns the point from which to start the DFS. - fn start_point(&self) -> Location; - - /// Returns true if the source region contains the given point. - fn source_region_contains(&mut self, point_index: RegionElementIndex) -> bool; - - /// Adds the given point to the target region, returning true if - /// something has changed. Returns `Err` if we should abort the - /// walk early. - fn add_to_target_region( - &mut self, - point_index: RegionElementIndex, - ) -> Result; - - /// Adds all universal regions in the source region to the target region, returning - /// true if something has changed. - fn add_universal_regions_outlived_by_source_to_target(&mut self) -> Result; -} - -/// Used during inference to enforce a `R1: R2 @ P` constraint. For -/// each point Q we reach along the DFS, we check if Q is in R2 (the -/// "source region"). If not, we stop the walk. Otherwise, we add Q to -/// R1 (the "target region") and continue to Q's successors. If we -/// reach the end of the graph, then we add any universal regions from -/// R2 into R1. -pub(super) struct CopyFromSourceToTarget<'v> { - pub source_region: RegionVid, - pub target_region: RegionVid, - pub inferred_values: &'v mut RegionValues, - pub constraint_point: Location, - pub constraint_span: Span, -} - -impl<'v> DfsOp for CopyFromSourceToTarget<'v> { - /// We never stop the walk early. - type Early = !; - - fn start_point(&self) -> Location { - self.constraint_point - } - - fn source_region_contains(&mut self, point_index: RegionElementIndex) -> bool { - self.inferred_values - .contains(self.source_region, point_index) - } - - fn add_to_target_region(&mut self, point_index: RegionElementIndex) -> Result { - Ok(self.inferred_values.add_due_to_outlives( - self.source_region, - self.target_region, - point_index, - self.constraint_point, - self.constraint_span, - )) - } - - fn add_universal_regions_outlived_by_source_to_target(&mut self) -> Result { - Ok(self.inferred_values.add_universal_regions_outlived_by( - self.source_region, - self.target_region, - self.constraint_point, - self.constraint_span, - )) - } -} - -/// Used after inference to *test* a `R1: R2 @ P` constraint. For -/// each point Q we reach along the DFS, we check if Q in R2 is also -/// contained in R1. If not, we abort the walk early with an `Err` -/// condition. Similarly, if we reach the end of the graph and find -/// that R1 contains some universal region that R2 does not contain, -/// we abort the walk early. -pub(super) struct TestTargetOutlivesSource<'v, 'tcx: 'v> { - pub source_region: RegionVid, - pub target_region: RegionVid, - pub elements: &'v RegionValueElements, - pub universal_regions: &'v UniversalRegions<'tcx>, - pub inferred_values: &'v RegionValues, - pub constraint_point: Location, -} - -impl<'v, 'tcx> DfsOp for TestTargetOutlivesSource<'v, 'tcx> { - /// The element that was not found within R2. - type Early = RegionElementIndex; - - fn start_point(&self) -> Location { - self.constraint_point - } - - fn source_region_contains(&mut self, point_index: RegionElementIndex) -> bool { - self.inferred_values - .contains(self.source_region, point_index) - } - - fn add_to_target_region( - &mut self, - point_index: RegionElementIndex, - ) -> Result { - if !self.inferred_values - .contains(self.target_region, point_index) - { - return Err(point_index); - } - - Ok(false) - } - - fn add_universal_regions_outlived_by_source_to_target( - &mut self, - ) -> Result { - // For all `ur_in_source` in `source_region`. - for ur_in_source in self.inferred_values - .universal_regions_outlived_by(self.source_region) - { - // Check that `target_region` outlives `ur_in_source`. - - // If `ur_in_source` is a member of `target_region`, OK. - // - // (This is implied by the loop below, actually, just an - // irresistible micro-opt. Mm. Premature optimization. So - // tasty.) - if self.inferred_values - .contains(self.target_region, ur_in_source) - { - continue; - } - - // If there is some other element X such that `target_region: X` and - // `X: ur_in_source`, OK. - if self.inferred_values - .universal_regions_outlived_by(self.target_region) - .any(|ur_in_target| self.universal_regions.outlives(ur_in_target, ur_in_source)) - { - continue; - } - - // Otherwise, not known to be true. - return Err(self.elements.index(ur_in_source)); - } - - Ok(false) - } -} diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 57b8824191f..5a1ab73b2b8 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -9,16 +9,19 @@ // except according to those terms. use super::universal_regions::UniversalRegions; +use borrow_check::nll::region_infer::values::ToElementIndex; use rustc::hir::def_id::DefId; +use rustc::infer::error_reporting::nice_region_error::NiceRegionError; +use rustc::infer::region_constraints::{GenericKind, VarInfos}; use rustc::infer::InferCtxt; use rustc::infer::NLLRegionVariableOrigin; use rustc::infer::RegionObligation; use rustc::infer::RegionVariableOrigin; use rustc::infer::SubregionOrigin; -use rustc::infer::error_reporting::nice_region_error::NiceRegionError; -use rustc::infer::region_constraints::{GenericKind, VarInfos}; -use rustc::mir::{ClosureOutlivesRequirement, ClosureOutlivesSubject, ClosureRegionRequirements, - Local, Location, Mir}; +use rustc::mir::{ + ClosureOutlivesRequirement, ClosureOutlivesSubject, ClosureRegionRequirements, Local, Location, + Mir, +}; use rustc::traits::ObligationCause; use rustc::ty::{self, RegionVid, Ty, TypeFoldable}; use rustc::util::common::{self, ErrorReported}; @@ -30,8 +33,6 @@ use syntax::ast; use syntax_pos::Span; mod annotation; -mod dfs; -use self::dfs::{CopyFromSourceToTarget, TestTargetOutlivesSource}; mod dump_mir; mod graphviz; mod values; @@ -100,7 +101,7 @@ struct RegionDefinition<'tcx> { /// NB: The variants in `Cause` are intentionally ordered. Lower /// values are preferred when it comes to error messages. Do not /// reorder willy nilly. -#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] pub(crate) enum Cause { /// point inserted because Local was live at the given Location LiveVar(Local, Location), @@ -114,23 +115,6 @@ pub(crate) enum Cause { /// part of the initial set of values for a universally quantified region UniversalRegion(RegionVid), - - /// Element E was added to R because there was some - /// outlives obligation `R: R1 @ P` and `R1` contained `E`. - Outlives { - /// the reason that R1 had E - original_cause: Rc, - - /// the point P from the relation - constraint_location: Location, - - /// The span indicating why we added the outlives constraint. - constraint_span: Span, - }, -} - -pub(crate) struct RegionCausalInfo { - inferred_values: RegionValues, } #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -325,7 +309,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { // Add all nodes in the CFG to liveness constraints for point_index in self.elements.all_point_indices() { - self.liveness_constraints.add( + self.liveness_constraints.add_element( variable, point_index, &Cause::UniversalRegion(variable), @@ -333,8 +317,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { } // Add `end(X)` into the set for X. - self.liveness_constraints - .add(variable, variable, &Cause::UniversalRegion(variable)); + self.liveness_constraints.add_element( + variable, + variable, + &Cause::UniversalRegion(variable), + ); } } @@ -383,7 +370,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { debug!("add_live_point: @{:?} Adding cause {:?}", point, cause); let element = self.elements.index(point); - if self.liveness_constraints.add(v, element, &cause) { + if self.liveness_constraints.add_element(v, element, &cause) { true } else { false @@ -438,9 +425,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { ) -> Option> { assert!(self.inferred_values.is_none(), "values already inferred"); - let dfs_storage = &mut self.new_dfs_storage(); - - self.propagate_constraints(mir, dfs_storage); + self.propagate_constraints(mir); // If this is a closure, we can propagate unsatisfied // `outlives_requirements` to our creator, so create a vector @@ -453,13 +438,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { None }; - self.check_type_tests( - infcx, - mir, - dfs_storage, - mir_def_id, - outlives_requirements.as_mut(), - ); + self.check_type_tests(infcx, mir, mir_def_id, outlives_requirements.as_mut()); self.check_universal_regions(infcx, mir_def_id, outlives_requirements.as_mut()); @@ -476,31 +455,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - /// Re-execute the region inference, this time tracking causal information. - /// This is significantly slower, so it is done only when an error is being reported. - pub(super) fn compute_causal_info(&self, mir: &Mir<'tcx>) -> RegionCausalInfo { - let dfs_storage = &mut self.new_dfs_storage(); - let inferred_values = self.compute_region_values(mir, dfs_storage, TrackCauses(true)); - RegionCausalInfo { inferred_values } - } - /// Propagate the region constraints: this will grow the values /// for each region variable until all the constraints are /// satisfied. Note that some values may grow **too** large to be /// feasible, but we check this later. - fn propagate_constraints(&mut self, mir: &Mir<'tcx>, dfs_storage: &mut dfs::DfsStorage) { + fn propagate_constraints(&mut self, mir: &Mir<'tcx>) { self.dependency_map = Some(self.build_dependency_map()); - let inferred_values = self.compute_region_values(mir, dfs_storage, TrackCauses(false)); + let inferred_values = self.compute_region_values(mir); self.inferred_values = Some(inferred_values); } #[inline(never)] // ensure dfs is identifiable in profiles - fn compute_region_values( - &self, - mir: &Mir<'tcx>, - dfs_storage: &mut dfs::DfsStorage, - track_causes: TrackCauses, - ) -> RegionValues { + fn compute_region_values(&self, _mir: &Mir<'tcx>) -> RegionValues { debug!("compute_region_values()"); debug!("compute_region_values: constraints={:#?}", { let mut constraints: Vec<_> = self.constraints.iter().collect(); @@ -510,7 +476,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { // The initial values for each region are derived from the liveness // constraints we have accumulated. - let mut inferred_values = self.liveness_constraints.duplicate(track_causes); + let mut inferred_values = self.liveness_constraints.duplicate(TrackCauses(false)); let dependency_map = self.dependency_map.as_ref().unwrap(); @@ -527,21 +493,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { let constraint = &self.constraints[constraint_idx]; debug!("propagate_constraints: constraint={:?}", constraint); - // Grow the value as needed to accommodate the - // outlives constraint. - let Ok(made_changes) = self.dfs( - mir, - dfs_storage, - CopyFromSourceToTarget { - source_region: constraint.sub, - target_region: constraint.sup, - inferred_values: &mut inferred_values, - constraint_point: constraint.point, - constraint_span: constraint.span, - }, - ); - - if made_changes { + if inferred_values.add_region(constraint.sup, constraint.sub) { debug!("propagate_constraints: sub={:?}", constraint.sub); debug!("propagate_constraints: sup={:?}", constraint.sup); @@ -586,7 +538,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { &self, infcx: &InferCtxt<'_, 'gcx, 'tcx>, mir: &Mir<'tcx>, - dfs_storage: &mut dfs::DfsStorage, mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, ) { @@ -595,13 +546,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { for type_test in &self.type_tests { debug!("check_type_test: {:?}", type_test); - if self.eval_region_test( - mir, - dfs_storage, - type_test.point, - type_test.lower_bound, - &type_test.test, - ) { + if self.eval_region_test(mir, type_test.point, type_test.lower_bound, &type_test.test) { continue; } @@ -858,7 +803,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { fn eval_region_test( &self, mir: &Mir<'tcx>, - dfs_storage: &mut dfs::DfsStorage, point: Location, lower_bound: RegionVid, test: &RegionTest, @@ -871,27 +815,26 @@ impl<'tcx> RegionInferenceContext<'tcx> { match test { RegionTest::IsOutlivedByAllRegionsIn(regions) => regions .iter() - .all(|&r| self.eval_outlives(mir, dfs_storage, r, lower_bound, point)), + .all(|&r| self.eval_outlives(mir, r, lower_bound, point)), RegionTest::IsOutlivedByAnyRegionIn(regions) => regions .iter() - .any(|&r| self.eval_outlives(mir, dfs_storage, r, lower_bound, point)), + .any(|&r| self.eval_outlives(mir, r, lower_bound, point)), RegionTest::Any(tests) => tests .iter() - .any(|test| self.eval_region_test(mir, dfs_storage, point, lower_bound, test)), + .any(|test| self.eval_region_test(mir, point, lower_bound, test)), RegionTest::All(tests) => tests .iter() - .all(|test| self.eval_region_test(mir, dfs_storage, point, lower_bound, test)), + .all(|test| self.eval_region_test(mir, point, lower_bound, test)), } } // Evaluate whether `sup_region: sub_region @ point`. fn eval_outlives( &self, - mir: &Mir<'tcx>, - dfs_storage: &mut dfs::DfsStorage, + _mir: &Mir<'tcx>, sup_region: RegionVid, sub_region: RegionVid, point: Location, @@ -901,36 +844,46 @@ impl<'tcx> RegionInferenceContext<'tcx> { sup_region, sub_region, point ); - // Roughly speaking, do a DFS of all region elements reachable - // from `point` contained in `sub_region`. If any of those are - // *not* present in `sup_region`, the DFS will abort early and - // yield an `Err` result. - match self.dfs( - mir, - dfs_storage, - TestTargetOutlivesSource { - source_region: sub_region, - target_region: sup_region, - constraint_point: point, - elements: &self.elements, - universal_regions: &self.universal_regions, - inferred_values: self.inferred_values.as_ref().unwrap(), - }, - ) { - Ok(_) => { - debug!("eval_outlives: true"); - true - } + let inferred_values = self.inferred_values + .as_ref() + .expect("values for regions not yet inferred"); - Err(elem) => { - debug!( - "eval_outlives: false because `{:?}` is not present in `{:?}`", - self.elements.to_element(elem), - sup_region - ); - false - } + debug!( + "eval_outlives: sup_region's value = {:?}", + inferred_values.region_value_str(sup_region), + ); + debug!( + "eval_outlives: sub_region's value = {:?}", + inferred_values.region_value_str(sub_region), + ); + + // Both the `sub_region` and `sup_region` consist of the union + // of some number of universal regions (along with the union + // of various points in the CFG; ignore those points for + // now). Therefore, the sup-region outlives the sub-region if, + // for each universal region R1 in the sub-region, there + // exists some region R2 in the sup-region that outlives R1. + let universal_outlives = inferred_values + .universal_regions_outlived_by(sub_region) + .all(|r1| { + inferred_values + .universal_regions_outlived_by(sup_region) + .any(|r2| self.universal_regions.outlives(r2, r1)) + }); + + if !universal_outlives { + return false; } + + // Now we have to compare all the points in the sub region and make + // sure they exist in the sup region. + + if self.universal_regions.is_universal_region(sup_region) { + // Micro-opt: universal regions contain all points. + return true; + } + + inferred_values.contains_points(sup_region, sub_region) } /// Once regions have been propagated, this method is used to see @@ -1007,7 +960,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { longer_fr, shorter_fr, ); - let blame_span = self.blame_span(longer_fr, shorter_fr); + let blame_index = self.blame_constraint(longer_fr, shorter_fr); + let blame_span = self.constraints[blame_index].span; if let Some(propagated_outlives_requirements) = propagated_outlives_requirements { // Shrink `fr` until we find a non-local region (if we do). @@ -1093,9 +1047,20 @@ impl<'tcx> RegionInferenceContext<'tcx> { diag.emit(); } + crate fn why_region_contains_point(&self, fr1: RegionVid, elem: Location) -> Option { + // Find some constraint `X: Y` where: + // - `fr1: X` transitively + // - and `Y` is live at `elem` + let index = self.blame_constraint(fr1, elem); + let region_sub = self.constraints[index].sub; + + // then return why `Y` was live at `elem` + self.liveness_constraints.cause(region_sub, elem) + } + /// Tries to finds a good span to blame for the fact that `fr1` /// contains `fr2`. - fn blame_span(&self, fr1: RegionVid, fr2: RegionVid) -> Span { + fn blame_constraint(&self, fr1: RegionVid, elem: impl ToElementIndex) -> ConstraintIndex { // Find everything that influenced final value of `fr`. let influenced_fr1 = self.dependencies(fr1); @@ -1108,23 +1073,23 @@ impl<'tcx> RegionInferenceContext<'tcx> { // of dependencies, which doesn't account for the locations of // contraints at all. But it will do for now. let relevant_constraint = self.constraints - .iter() - .filter_map(|constraint| { - if constraint.sub != fr2 { - None - } else { - influenced_fr1[constraint.sup] - .map(|distance| (distance, constraint.span)) - } - }) - .min() // constraining fr1 with fewer hops *ought* to be more obvious - .map(|(_dist, span)| span); + .iter_enumerated() + .filter_map(|(i, constraint)| { + if !self.liveness_constraints.contains(constraint.sub, elem) { + None + } else { + influenced_fr1[constraint.sup] + .map(|distance| (distance, i)) + } + }) + .min() // constraining fr1 with fewer hops *ought* to be more obvious + .map(|(_dist, i)| i); relevant_constraint.unwrap_or_else(|| { bug!( "could not find any constraint to blame for {:?}: {:?}", fr1, - fr2 + elem, ); }) } @@ -1161,16 +1126,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } -impl RegionCausalInfo { - /// Returns the *reason* that the region `r` contains the given point. - pub(super) fn why_region_contains_point(&self, r: R, p: Location) -> Option> - where - R: ToRegionVid, - { - self.inferred_values.cause(r.to_region_vid(), p) - } -} - impl<'tcx> RegionDefinition<'tcx> { fn new(origin: RegionVariableOrigin) -> Self { // Create a new region definition. Note that, for free @@ -1314,31 +1269,3 @@ impl<'gcx, 'tcx> ClosureRegionRequirementsExt<'gcx, 'tcx> for ClosureRegionRequi }) } } - -trait CauseExt { - fn outlives(&self, constraint_location: Location, constraint_span: Span) -> Cause; -} - -impl CauseExt for Rc { - /// Creates a derived cause due to an outlives constraint. - fn outlives(&self, constraint_location: Location, constraint_span: Span) -> Cause { - Cause::Outlives { - original_cause: self.clone(), - constraint_location, - constraint_span, - } - } -} - -impl Cause { - pub(crate) fn root_cause(&self) -> &Cause { - match self { - Cause::LiveVar(..) - | Cause::DropVar(..) - | Cause::LiveOther(..) - | Cause::UniversalRegion(..) => self, - - Cause::Outlives { original_cause, .. } => original_cause.root_cause(), - } - } -} diff --git a/src/librustc_mir/borrow_check/nll/region_infer/values.rs b/src/librustc_mir/borrow_check/nll/region_infer/values.rs index d15d85792d9..e914be52db0 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/values.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/values.rs @@ -8,16 +8,17 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::rc::Rc; +use borrow_check::nll::region_infer::TrackCauses; +use rustc::mir::{BasicBlock, Location, Mir}; +use rustc::ty::RegionVid; use rustc_data_structures::bitvec::SparseBitMatrix; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::indexed_vec::IndexVec; -use rustc::mir::{BasicBlock, Location, Mir}; -use rustc::ty::{self, RegionVid}; -use syntax::codemap::Span; +use std::fmt::Debug; +use std::rc::Rc; -use super::{Cause, CauseExt, TrackCauses}; +use super::Cause; /// Maps between the various kinds of elements of a region value to /// the internal indices that w use. @@ -72,11 +73,6 @@ impl RegionValueElements { (0..self.num_points).map(move |i| RegionElementIndex::new(i + self.num_universal_regions)) } - /// Iterates over the `RegionElementIndex` for all points in the CFG. - pub(super) fn all_universal_region_indices(&self) -> impl Iterator { - (0..self.num_universal_regions).map(move |i| RegionElementIndex::new(i)) - } - /// Converts a particular `RegionElementIndex` to the `RegionElement` it represents. pub(super) fn to_element(&self, i: RegionElementIndex) -> RegionElement { debug!("to_element(i={:?})", i); @@ -152,7 +148,7 @@ pub(super) enum RegionElement { UniversalRegion(RegionVid), } -pub(super) trait ToElementIndex { +pub(super) trait ToElementIndex: Debug + Copy { fn to_element_index(self, elements: &RegionValueElements) -> RegionElementIndex; } @@ -195,7 +191,7 @@ pub(super) struct RegionValues { causes: Option, } -type CauseMap = FxHashMap<(RegionVid, RegionElementIndex), Rc>; +type CauseMap = FxHashMap<(RegionVid, RegionElementIndex), Cause>; impl RegionValues { /// Creates a new set of "region values" that tracks causal information. @@ -237,11 +233,22 @@ impl RegionValues { /// Adds the given element to the value for the given region. Returns true if /// the element is newly added (i.e., was not already present). - pub(super) fn add(&mut self, r: RegionVid, elem: E, cause: &Cause) -> bool { + pub(super) fn add_element( + &mut self, + r: RegionVid, + elem: E, + cause: &Cause, + ) -> bool { let i = self.elements.index(elem); self.add_internal(r, i, |_| cause.clone()) } + /// Add all elements in `r_from` to `r_to` (because e.g. `r_to: + /// r_from`). + pub(super) fn add_region(&mut self, r_to: RegionVid, r_from: RegionVid) -> bool { + self.matrix.merge(r_from, r_to) + } + /// Internal method to add an element to a region. /// /// Takes a "lazy" cause -- this function will return the cause, but it will only @@ -254,7 +261,7 @@ impl RegionValues { debug!("add(r={:?}, i={:?})", r, self.elements.to_element(i)); if let Some(causes) = &mut self.causes { - let cause = Rc::new(make_cause(causes)); + let cause = make_cause(causes); causes.insert((r, i), cause); } @@ -266,15 +273,8 @@ impl RegionValues { // #49998: compare using root cause alone to avoid // useless traffic from similar outlives chains. - let overwrite = if ty::tls::with(|tcx| { - tcx.sess.opts.debugging_opts.nll_subminimal_causes - }) { - cause.root_cause() < old_cause.root_cause() - } else { - cause < **old_cause - }; - if overwrite { - *old_cause = Rc::new(cause); + if cause < *old_cause { + *old_cause = cause; return true; } } @@ -283,62 +283,22 @@ impl RegionValues { } } - /// Adds `elem` to `to_region` because of a relation: - /// - /// to_region: from_region @ constraint_location - /// - /// that was added by the cod at `constraint_span`. - pub(super) fn add_due_to_outlives( - &mut self, - from_region: RegionVid, - to_region: RegionVid, - elem: T, - constraint_location: Location, - constraint_span: Span, - ) -> bool { - let elem = self.elements.index(elem); - self.add_internal(to_region, elem, |causes| { - causes[&(from_region, elem)].outlives(constraint_location, constraint_span) - }) - } - - /// Adds all the universal regions outlived by `from_region` to - /// `to_region`. - pub(super) fn add_universal_regions_outlived_by( - &mut self, - from_region: RegionVid, - to_region: RegionVid, - constraint_location: Location, - constraint_span: Span, - ) -> bool { - // We could optimize this by improving `SparseBitMatrix::merge` so - // it does not always merge an entire row. That would - // complicate causal tracking though. - debug!( - "add_universal_regions_outlived_by(from_region={:?}, to_region={:?})", - from_region, to_region - ); - let mut changed = false; - for elem in self.elements.all_universal_region_indices() { - if self.contains(from_region, elem) { - changed |= self.add_due_to_outlives( - from_region, - to_region, - elem, - constraint_location, - constraint_span, - ); - } - } - changed - } - /// True if the region `r` contains the given element. pub(super) fn contains(&self, r: RegionVid, elem: E) -> bool { let i = self.elements.index(elem); self.matrix.contains(r, i) } + /// True if `sup_region` contains all the CFG points that + /// `sub_region` contains. Ignores universal regions. + pub(super) fn contains_points(&self, sup_region: RegionVid, sub_region: RegionVid) -> bool { + // This could be done faster by comparing the bitsets. But I + // am lazy. + self.element_indices_contained_in(sub_region) + .skip_while(|&i| self.elements.to_universal_region(i).is_some()) + .all(|e| self.contains(sup_region, e)) + } + /// Iterate over the value of the region `r`, yielding up element /// indices. You may prefer `universal_regions_outlived_by` or /// `elements_contained_in`. @@ -444,7 +404,7 @@ impl RegionValues { /// /// Returns None if cause tracking is disabled or `elem` is not /// actually found in `r`. - pub(super) fn cause(&self, r: RegionVid, elem: T) -> Option> { + pub(super) fn cause(&self, r: RegionVid, elem: T) -> Option { let index = self.elements.index(elem); if let Some(causes) = &self.causes { causes.get(&(r, index)).cloned() diff --git a/src/test/mir-opt/nll/region-liveness-two-disjoint-uses.rs b/src/test/mir-opt/nll/region-liveness-two-disjoint-uses.rs index 153739133ac..62064fa94f2 100644 --- a/src/test/mir-opt/nll/region-liveness-two-disjoint-uses.rs +++ b/src/test/mir-opt/nll/region-liveness-two-disjoint-uses.rs @@ -13,6 +13,9 @@ // borrows in `&v[0]` and `&v[1]` each (in theory) have to outlive R3, // but only at a particular point, and hence they wind up including // distinct regions. +// +// FIXME(#43234) -- Well, this used to be true, but we modified NLL +// for the time being to not take location into account. // compile-flags:-Zborrowck=mir -Zverbose // ^^^^^^^^^ force compiler to dump more region information @@ -36,9 +39,9 @@ fn main() { // END RUST SOURCE // START rustc.main.nll.0.mir -// | '_#2r | {bb2[0..=1], bb3[0..=1]} +// | '_#2r | {bb2[0..=1], bb3[0..=1], bb8[2..=4]} // ... -// | '_#4r | {bb8[1..=4]} +// | '_#4r | {bb2[1], bb3[0..=1], bb8[1..=4]} // | '_#5r | {bb2[1], bb3[0..=1], bb8[2..=4]} // ... // let mut _2: &'_#5r usize; diff --git a/src/test/run-pass/nll/get_default.rs b/src/test/run-pass/nll/get_default.rs deleted file mode 100644 index 13ef907d8d0..00000000000 --- a/src/test/run-pass/nll/get_default.rs +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2016 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -#![feature(nll)] - -use std::collections::HashMap; - -fn get_default(map: &mut HashMap, key: usize) -> &mut String { - match map.get_mut(&key) { - Some(value) => value, - None => { - map.insert(key, "".to_string()); - map.get_mut(&key).unwrap() - } - } -} - -fn main() { - let map = &mut HashMap::new(); - map.insert(22, format!("Hello, world")); - map.insert(44, format!("Goodbye, world")); - assert_eq!(&*get_default(map, 22), "Hello, world"); - assert_eq!(&*get_default(map, 66), ""); -} diff --git a/src/test/ui/borrowck/mut-borrow-in-loop.nll.stderr b/src/test/ui/borrowck/mut-borrow-in-loop.nll.stderr index fc288e6b1d6..2284f0784c5 100644 --- a/src/test/ui/borrowck/mut-borrow-in-loop.nll.stderr +++ b/src/test/ui/borrowck/mut-borrow-in-loop.nll.stderr @@ -2,19 +2,28 @@ error[E0499]: cannot borrow `*arg` as mutable more than once at a time --> $DIR/mut-borrow-in-loop.rs:20:25 | LL | (self.func)(arg) //~ ERROR cannot borrow - | ^^^ mutable borrow starts here in previous iteration of loop + | ------------^^^- + | | | + | | mutable borrow starts here in previous iteration of loop + | borrow later used here error[E0499]: cannot borrow `*arg` as mutable more than once at a time --> $DIR/mut-borrow-in-loop.rs:26:25 | LL | (self.func)(arg) //~ ERROR cannot borrow - | ^^^ mutable borrow starts here in previous iteration of loop + | ------------^^^- + | | | + | | mutable borrow starts here in previous iteration of loop + | borrow later used here error[E0499]: cannot borrow `*arg` as mutable more than once at a time --> $DIR/mut-borrow-in-loop.rs:33:25 | LL | (self.func)(arg) //~ ERROR cannot borrow - | ^^^ mutable borrow starts here in previous iteration of loop + | ------------^^^- + | | | + | | mutable borrow starts here in previous iteration of loop + | borrow later used here error: aborting due to 3 previous errors diff --git a/src/test/ui/nll/get_default.nll.stderr b/src/test/ui/nll/get_default.nll.stderr index c6f021f8c36..b955a51e38d 100644 --- a/src/test/ui/nll/get_default.nll.stderr +++ b/src/test/ui/nll/get_default.nll.stderr @@ -4,14 +4,14 @@ error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as imm LL | match map.get() { | --- immutable borrow occurs here ... -LL | map.set(String::new()); // Just AST errors here +LL | map.set(String::new()); // Ideally, this would not error. | ^^^ mutable borrow occurs here ... LL | } | - immutable borrow ends here error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/get_default.rs:44:17 + --> $DIR/get_default.rs:45:17 | LL | match map.get() { | --- immutable borrow occurs here @@ -23,19 +23,40 @@ LL | } | - immutable borrow ends here error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/get_default.rs:50:17 + --> $DIR/get_default.rs:51:17 | LL | match map.get() { | --- immutable borrow occurs here ... -LL | map.set(String::new()); // Just AST errors here +LL | map.set(String::new()); // Ideally, just AST would error here | ^^^ mutable borrow occurs here ... LL | } | - immutable borrow ends here error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/get_default.rs:44:17 + --> $DIR/get_default.rs:33:17 + | +LL | match map.get() { + | --- immutable borrow occurs here +... +LL | map.set(String::new()); // Ideally, this would not error. + | ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here + | +note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 26:1... + --> $DIR/get_default.rs:26:1 + | +LL | / fn ok(map: &mut Map) -> &String { +LL | | loop { +LL | | match map.get() { +LL | | Some(v) => { +... | +LL | | } +LL | | } + | |_^ + +error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) + --> $DIR/get_default.rs:45:17 | LL | match map.get() { | --- immutable borrow occurs here @@ -46,6 +67,27 @@ LL | map.set(String::new()); // Both AST and MIR error here LL | return v; | - borrow later used here -error: aborting due to 4 previous errors +error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) + --> $DIR/get_default.rs:51:17 + | +LL | match map.get() { + | --- immutable borrow occurs here +... +LL | map.set(String::new()); // Ideally, just AST would error here + | ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here + | +note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 41:1... + --> $DIR/get_default.rs:41:1 + | +LL | / fn err(map: &mut Map) -> &String { +LL | | loop { +LL | | match map.get() { +LL | | Some(v) => { +... | +LL | | } +LL | | } + | |_^ + +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0502`. diff --git a/src/test/ui/nll/get_default.rs b/src/test/ui/nll/get_default.rs index 728c84695ea..1a417b1e28c 100644 --- a/src/test/ui/nll/get_default.rs +++ b/src/test/ui/nll/get_default.rs @@ -30,8 +30,9 @@ fn ok(map: &mut Map) -> &String { return v; } None => { - map.set(String::new()); // Just AST errors here + map.set(String::new()); // Ideally, this would not error. //~^ ERROR borrowed as immutable (Ast) + //~| ERROR borrowed as immutable (Mir) } } } @@ -47,8 +48,9 @@ fn err(map: &mut Map) -> &String { return v; } None => { - map.set(String::new()); // Just AST errors here + map.set(String::new()); // Ideally, just AST would error here //~^ ERROR borrowed as immutable (Ast) + //~| ERROR borrowed as immutable (Mir) } } } diff --git a/src/test/ui/nll/get_default.stderr b/src/test/ui/nll/get_default.stderr index 064fd38b872..dd69e18652c 100644 --- a/src/test/ui/nll/get_default.stderr +++ b/src/test/ui/nll/get_default.stderr @@ -4,14 +4,14 @@ error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as imm LL | match map.get() { | --- immutable borrow occurs here ... -LL | map.set(String::new()); // Just AST errors here +LL | map.set(String::new()); // Ideally, this would not error. | ^^^ mutable borrow occurs here ... LL | } | - immutable borrow ends here error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/get_default.rs:44:17 + --> $DIR/get_default.rs:45:17 | LL | match map.get() { | --- immutable borrow occurs here @@ -23,19 +23,61 @@ LL | } | - immutable borrow ends here error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/get_default.rs:50:17 + --> $DIR/get_default.rs:51:17 | LL | match map.get() { | --- immutable borrow occurs here ... -LL | map.set(String::new()); // Just AST errors here +LL | map.set(String::new()); // Ideally, just AST would error here | ^^^ mutable borrow occurs here ... LL | } | - immutable borrow ends here error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/get_default.rs:44:17 + --> $DIR/get_default.rs:33:17 + | +LL | match map.get() { + | --- immutable borrow occurs here +... +LL | map.set(String::new()); // Ideally, this would not error. + | ^^^ mutable borrow occurs here + | +note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 26:1... + --> $DIR/get_default.rs:26:1 + | +LL | / fn ok(map: &mut Map) -> &String { +LL | | loop { +LL | | match map.get() { +LL | | Some(v) => { +... | +LL | | } +LL | | } + | |_^ + +error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) + --> $DIR/get_default.rs:51:17 + | +LL | match map.get() { + | --- immutable borrow occurs here +... +LL | map.set(String::new()); // Ideally, just AST would error here + | ^^^ mutable borrow occurs here + | +note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 41:1... + --> $DIR/get_default.rs:41:1 + | +LL | / fn err(map: &mut Map) -> &String { +LL | | loop { +LL | | match map.get() { +LL | | Some(v) => { +... | +LL | | } +LL | | } + | |_^ + +error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) + --> $DIR/get_default.rs:45:17 | LL | match map.get() { | --- immutable borrow occurs here @@ -46,6 +88,6 @@ LL | map.set(String::new()); // Both AST and MIR error here LL | return v; | - borrow later used here -error: aborting due to 4 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0502`.