Skip to content

Commit

Permalink
Auto merge of #53327 - wesleywiser:wip_optimize_nll, r=nikomatsakis
Browse files Browse the repository at this point in the history
[nll] teach SCC about `'static`

r? @nikomatsakis

I think this is right? I am seeing better performance on the `html5ever` benchmark but I'd like a perf run to quantify the exact speedup. There's a few ui tests failing due to changes in the error messages. The main issue seems to be that returns aren't being detected correctly?

`mir_check_cast_unsize.rs` before:

```
error: unsatisfied lifetime constraints
  --> mir_check_cast_unsize.rs:17:46
   |
17 |   fn bar<'a>(x: &'a u32) -> &'static dyn Debug {
   |  ________--____________________________________^
   | |        |
   | |        lifetime `'a` defined here
18 | |     //~^ ERROR unsatisfied lifetime constraints
19 | |     x
20 | |     //~^ WARNING not reporting region error due to nll
21 | | }
   | |_^ return requires that `'a` must outlive `'static`
```

`mir_check_cast_unsize.rs` after:

```
error: unsatisfied lifetime constraints
  --> mir_check_cast_unsize.rs:19:5
   |
17 | fn bar<'a>(x: &'a u32) -> &'static dyn Debug {
   |        -- lifetime `'a` defined here
18 |     //~^ ERROR unsatisfied lifetime constraints
19 |     x
   |     ^ cast requires that `'a` must outlive `'static`
```
  • Loading branch information
bors committed Sep 7, 2018
2 parents 24ef47b + b1211e8 commit fc81e36
Show file tree
Hide file tree
Showing 39 changed files with 376 additions and 190 deletions.
7 changes: 7 additions & 0 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2567,6 +2567,13 @@ fn insert_late_bound_lifetimes(
// - do not appear in the where-clauses
// - are not implicitly captured by `impl Trait`
for param in &generics.params {
match param.kind {
hir::GenericParamKind::Lifetime { .. } => { /* fall through */ }

// Types are not late-bound.
hir::GenericParamKind::Type { .. } => continue,
}

let lt_name = hir::LifetimeName::Param(param.name.modern());
// appears in the where clauses? early-bound.
if appears_in_where_clause.regions.contains(&lt_name) {
Expand Down
84 changes: 70 additions & 14 deletions src/librustc_mir/borrow_check/nll/constraints/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use borrow_check::nll::type_check::Locations;
use borrow_check::nll::constraints::{ConstraintIndex, ConstraintSet, OutlivesConstraint};
use rustc::ty::RegionVid;
use rustc_data_structures::graph;
Expand All @@ -31,6 +32,7 @@ crate type ReverseConstraintGraph = ConstraintGraph<Reverse>;
crate trait ConstraintGraphDirecton: Copy + 'static {
fn start_region(c: &OutlivesConstraint) -> RegionVid;
fn end_region(c: &OutlivesConstraint) -> RegionVid;
fn is_normal() -> bool;
}

/// In normal mode, a `R1: R2` constraint results in an edge `R1 ->
Expand All @@ -48,6 +50,10 @@ impl ConstraintGraphDirecton for Normal {
fn end_region(c: &OutlivesConstraint) -> RegionVid {
c.sub
}

fn is_normal() -> bool {
true
}
}

/// In reverse mode, a `R1: R2` constraint results in an edge `R2 ->
Expand All @@ -65,6 +71,10 @@ impl ConstraintGraphDirecton for Reverse {
fn end_region(c: &OutlivesConstraint) -> RegionVid {
c.sup
}

fn is_normal() -> bool {
false
}
}

impl<D: ConstraintGraphDirecton> ConstraintGraph<D> {
Expand Down Expand Up @@ -98,32 +108,74 @@ impl<D: ConstraintGraphDirecton> ConstraintGraph<D> {
/// Given the constraint set from which this graph was built
/// creates a region graph so that you can iterate over *regions*
/// and not constraints.
crate fn region_graph<'rg>(&'rg self, set: &'rg ConstraintSet) -> RegionGraph<'rg, D> {
RegionGraph::new(set, self)
crate fn region_graph<'rg>(
&'rg self,
set: &'rg ConstraintSet,
static_region: RegionVid,
) -> RegionGraph<'rg, D> {
RegionGraph::new(set, self, static_region)
}

/// Given a region `R`, iterate over all constraints `R: R1`.
crate fn outgoing_edges(&self, region_sup: RegionVid) -> Edges<'_, D> {
let first = self.first_constraints[region_sup];
Edges {
graph: self,
pointer: first,
crate fn outgoing_edges<'a>(
&'a self,
region_sup: RegionVid,
constraints: &'a ConstraintSet,
static_region: RegionVid,
) -> Edges<'a, D> {
//if this is the `'static` region and the graph's direction is normal,
//then setup the Edges iterator to return all regions #53178
if region_sup == static_region && D::is_normal() {
Edges {
graph: self,
constraints,
pointer: None,
next_static_idx: Some(0),
static_region,
}
} else {
//otherwise, just setup the iterator as normal
let first = self.first_constraints[region_sup];
Edges {
graph: self,
constraints,
pointer: first,
next_static_idx: None,
static_region,
}
}
}
}

crate struct Edges<'s, D: ConstraintGraphDirecton> {
graph: &'s ConstraintGraph<D>,
constraints: &'s ConstraintSet,
pointer: Option<ConstraintIndex>,
next_static_idx: Option<usize>,
static_region: RegionVid,
}

impl<'s, D: ConstraintGraphDirecton> Iterator for Edges<'s, D> {
type Item = ConstraintIndex;
type Item = OutlivesConstraint;

fn next(&mut self) -> Option<Self::Item> {
if let Some(p) = self.pointer {
self.pointer = self.graph.next_constraints[p];
Some(p)

Some(self.constraints[p])
} else if let Some(next_static_idx) = self.next_static_idx {
self.next_static_idx =
if next_static_idx == (self.graph.first_constraints.len() - 1) {
None
} else {
Some(next_static_idx + 1)
};

Some(OutlivesConstraint {
sup: self.static_region,
sub: next_static_idx.into(),
locations: Locations::All,
})
} else {
None
}
Expand All @@ -136,40 +188,44 @@ impl<'s, D: ConstraintGraphDirecton> Iterator for Edges<'s, D> {
crate struct RegionGraph<'s, D: ConstraintGraphDirecton> {
set: &'s ConstraintSet,
constraint_graph: &'s ConstraintGraph<D>,
static_region: RegionVid,
}

impl<'s, D: ConstraintGraphDirecton> RegionGraph<'s, D> {
/// Create a "dependency graph" where each region constraint `R1:
/// R2` is treated as an edge `R1 -> R2`. We use this graph to
/// construct SCCs for region inference but also for error
/// reporting.
crate fn new(set: &'s ConstraintSet, constraint_graph: &'s ConstraintGraph<D>) -> Self {
crate fn new(
set: &'s ConstraintSet,
constraint_graph: &'s ConstraintGraph<D>,
static_region: RegionVid,
) -> Self {
Self {
set,
constraint_graph,
static_region,
}
}

/// Given a region `R`, iterate over all regions `R1` such that
/// there exists a constraint `R: R1`.
crate fn outgoing_regions(&self, region_sup: RegionVid) -> Successors<'_, D> {
Successors {
set: self.set,
edges: self.constraint_graph.outgoing_edges(region_sup),
edges: self.constraint_graph.outgoing_edges(region_sup, self.set, self.static_region),
}
}
}

crate struct Successors<'s, D: ConstraintGraphDirecton> {
set: &'s ConstraintSet,
edges: Edges<'s, D>,
}

impl<'s, D: ConstraintGraphDirecton> Iterator for Successors<'s, D> {
type Item = RegionVid;

fn next(&mut self) -> Option<Self::Item> {
self.edges.next().map(|c| D::end_region(&self.set[c]))
self.edges.next().map(|c| D::end_region(&c))
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/borrow_check/nll/constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ impl ConstraintSet {
crate fn compute_sccs(
&self,
constraint_graph: &graph::NormalConstraintGraph,
static_region: RegionVid,
) -> Sccs<RegionVid, ConstraintSccIndex> {
let region_graph = &constraint_graph.region_graph(self);
let region_graph = &constraint_graph.region_graph(self, static_region);
Sccs::new(region_graph)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use borrow_check::nll::region_infer::{ConstraintIndex, RegionInferenceContext};
use borrow_check::nll::constraints::OutlivesConstraint;
use borrow_check::nll::region_infer::RegionInferenceContext;
use borrow_check::nll::type_check::Locations;
use rustc::hir::def_id::DefId;
use rustc::infer::error_reporting::nice_region_error::NiceRegionError;
Expand Down Expand Up @@ -53,7 +54,7 @@ impl fmt::Display for ConstraintCategory {
#[derive(Copy, Clone, PartialEq, Eq)]
enum Trace {
StartRegion,
FromConstraint(ConstraintIndex),
FromOutlivesConstraint(OutlivesConstraint),
NotVisited,
}

Expand All @@ -80,12 +81,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
debug!(
"best_blame_constraint: path={:#?}",
path.iter()
.map(|&ci| format!(
"{:?}: {:?} ({:?}: {:?})",
ci,
&self.constraints[ci],
self.constraint_sccs.scc(self.constraints[ci].sup),
self.constraint_sccs.scc(self.constraints[ci].sub),
.map(|&c| format!(
"{:?} ({:?}: {:?})",
c,
self.constraint_sccs.scc(c.sup),
self.constraint_sccs.scc(c.sub),
))
.collect::<Vec<_>>()
);
Expand Down Expand Up @@ -121,7 +121,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// highlight (e.g., a call site or something).
let target_scc = self.constraint_sccs.scc(target_region);
let best_choice = (0..path.len()).rev().find(|&i| {
let constraint = &self.constraints[path[i]];
let constraint = path[i];

let constraint_sup_scc = self.constraint_sccs.scc(constraint.sup);

Expand Down Expand Up @@ -164,7 +164,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
&self,
from_region: RegionVid,
target_test: impl Fn(RegionVid) -> bool,
) -> Option<(Vec<ConstraintIndex>, RegionVid)> {
) -> Option<(Vec<OutlivesConstraint>, RegionVid)> {
let mut context = IndexVec::from_elem(Trace::NotVisited, &self.definitions);
context[from_region] = Trace::StartRegion;

Expand All @@ -185,9 +185,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
Trace::NotVisited => {
bug!("found unvisited region {:?} on path to {:?}", p, r)
}
Trace::FromConstraint(c) => {
Trace::FromOutlivesConstraint(c) => {
result.push(c);
p = self.constraints[c].sup;
p = c.sup;
}

Trace::StartRegion => {
Expand All @@ -201,11 +201,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// Otherwise, walk over the outgoing constraints and
// enqueue any regions we find, keeping track of how we
// reached them.
for constraint in self.constraint_graph.outgoing_edges(r) {
assert_eq!(self.constraints[constraint].sup, r);
let sub_region = self.constraints[constraint].sub;
let fr_static = self.universal_regions.fr_static;
for constraint in self.constraint_graph.outgoing_edges(r,
&self.constraints,
fr_static) {
assert_eq!(constraint.sup, r);
let sub_region = constraint.sub;
if let Trace::NotVisited = context[sub_region] {
context[sub_region] = Trace::FromConstraint(constraint);
context[sub_region] = Trace::FromOutlivesConstraint(constraint);
deque.push_back(sub_region);
}
}
Expand All @@ -216,8 +219,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {

/// This function will return true if a constraint is interesting and false if a constraint
/// is not. It is useful in filtering constraint paths to only interesting points.
fn constraint_is_interesting(&self, index: ConstraintIndex) -> bool {
let constraint = self.constraints[index];
fn constraint_is_interesting(&self, constraint: OutlivesConstraint) -> bool {
debug!(
"constraint_is_interesting: locations={:?} constraint={:?}",
constraint.locations, constraint
Expand All @@ -232,19 +234,18 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// This function classifies a constraint from a location.
fn classify_constraint(
&self,
index: ConstraintIndex,
constraint: OutlivesConstraint,
mir: &Mir<'tcx>,
tcx: TyCtxt<'_, '_, 'tcx>,
) -> (ConstraintCategory, Span) {
let constraint = self.constraints[index];
debug!("classify_constraint: constraint={:?}", constraint);
let span = constraint.locations.span(mir);
let location = constraint
.locations
.from_location()
.unwrap_or(Location::START);

if !self.constraint_is_interesting(index) {
if !self.constraint_is_interesting(constraint) {
return (ConstraintCategory::Boring, span);
}

Expand Down
Loading

0 comments on commit fc81e36

Please sign in to comment.