Skip to content

Commit

Permalink
Expand the "givens" set to cover transitive relations. The givens array
Browse files Browse the repository at this point in the history
stores relationships like `'c <= '0` (where `'c` is a free region and
`'0` is an inference variable) that are derived from closure
arguments. These are (rather hackily) ignored for purposes of inference,
preventing spurious errors. The current code did not handle transitive
cases like `'c <= '0` and `'0 <= '1`. Fixes rust-lang#24085.
  • Loading branch information
nikomatsakis committed Apr 17, 2015
1 parent a691f1e commit 11720a1
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 15 deletions.
3 changes: 2 additions & 1 deletion src/librustc/middle/cfg/mod.rs
Expand Up @@ -62,6 +62,7 @@ impl CFG {
}

pub fn node_is_reachable(&self, id: ast::NodeId) -> bool {
self.graph.depth_traverse(self.entry).any(|node| node.id() == id)
self.graph.depth_traverse(self.entry)
.any(|idx| self.graph.node_data(idx).id() == id)
}
}
6 changes: 3 additions & 3 deletions src/librustc/middle/graph.rs
Expand Up @@ -304,9 +304,9 @@ pub struct DepthFirstTraversal<'g, N:'g, E:'g> {
}

impl<'g, N, E> Iterator for DepthFirstTraversal<'g, N, E> {
type Item = &'g N;
type Item = NodeIndex;

fn next(&mut self) -> Option<&'g N> {
fn next(&mut self) -> Option<NodeIndex> {
while let Some(idx) = self.stack.pop() {
if !self.visited.insert(idx.node_id()) {
continue;
Expand All @@ -318,7 +318,7 @@ impl<'g, N, E> Iterator for DepthFirstTraversal<'g, N, E> {
true
});

return Some(self.graph.node_data(idx));
return Some(idx);
}

return None;
Expand Down
43 changes: 33 additions & 10 deletions src/librustc/middle/infer/region_inference/mod.rs
Expand Up @@ -978,13 +978,17 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {

// Dorky hack to cause `dump_constraints` to only get called
// if debug mode is enabled:
debug!("----() End constraint listing {:?}---", self.dump_constraints());
debug!("----() End constraint listing (subject={}) {:?}---",
subject, self.dump_constraints(subject));
graphviz::maybe_print_constraints_for(self, subject);

let graph = self.construct_graph();
self.expand_givens(&graph);
self.expansion(&mut var_data);
self.contraction(&mut var_data);
let values =
self.extract_values_and_collect_conflicts(&var_data[..],
&graph,
errors);
self.collect_concrete_region_errors(&values, errors);
values
Expand All @@ -1003,13 +1007,38 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {
}).collect()
}

fn dump_constraints(&self) {
debug!("----() Start constraint listing ()----");
fn dump_constraints(&self, subject: ast::NodeId) {
debug!("----() Start constraint listing (subject={}) ()----", subject);
for (idx, (constraint, _)) in self.constraints.borrow().iter().enumerate() {
debug!("Constraint {} => {}", idx, constraint.repr(self.tcx));
}
}

fn expand_givens(&self, graph: &RegionGraph) {
// Givens are a kind of horrible hack to account for
// constraints like 'c <= '0 that are known to hold due to
// closure signatures (see the comment above on the `givens`
// field). They should go away. But until they do, the role
// of this fn is to account for the transitive nature:
//
// Given 'c <= '0
// and '0 <= '1
// then 'c <= '1

let mut givens = self.givens.borrow_mut();
let seeds: Vec<_> = givens.iter().cloned().collect();
for (fr, vid) in seeds {
let seed_index = NodeIndex(vid.index as usize);
for succ_index in graph.depth_traverse(seed_index) {
let succ_index = succ_index.0 as u32;
if succ_index < self.num_vars() {
let succ_vid = RegionVid { index: succ_index };
givens.insert((fr, succ_vid));
}
}
}
}

fn expansion(&self, var_data: &mut [VarData]) {
self.iterate_until_fixed_point("Expansion", |constraint| {
debug!("expansion: constraint={} origin={}",
Expand Down Expand Up @@ -1241,6 +1270,7 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {
fn extract_values_and_collect_conflicts(
&self,
var_data: &[VarData],
graph: &RegionGraph,
errors: &mut Vec<RegionResolutionError<'tcx>>)
-> Vec<VarValue>
{
Expand All @@ -1259,8 +1289,6 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {
// overlapping locations.
let mut dup_vec: Vec<_> = repeat(u32::MAX).take(self.num_vars() as usize).collect();

let mut opt_graph = None;

for idx in 0..self.num_vars() as usize {
match var_data[idx].value {
Value(_) => {
Expand Down Expand Up @@ -1296,11 +1324,6 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {
starts to create problems we'll have to revisit
this portion of the code and think hard about it. =) */

if opt_graph.is_none() {
opt_graph = Some(self.construct_graph());
}
let graph = opt_graph.as_ref().unwrap();

let node_vid = RegionVid { index: idx as u32 };
match var_data[idx].classification {
Expanding => {
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_trans/trans/base.rs
Expand Up @@ -1110,7 +1110,8 @@ fn build_cfg(tcx: &ty::ctxt, id: ast::NodeId) -> (ast::NodeId, Option<cfg::CFG>)
// return slot alloca. This can cause errors related to clean-up due to
// the clobbering of the existing value in the return slot.
fn has_nested_returns(tcx: &ty::ctxt, cfg: &cfg::CFG, blk_id: ast::NodeId) -> bool {
for n in cfg.graph.depth_traverse(cfg.entry) {
for index in cfg.graph.depth_traverse(cfg.entry) {
let n = cfg.graph.node_data(index);
match tcx.map.find(n.id()) {
Some(ast_map::NodeExpr(ex)) => {
if let ast::ExprRet(Some(ref ret_expr)) = ex.node {
Expand Down
27 changes: 27 additions & 0 deletions src/test/run-pass/issue-24085.rs
@@ -0,0 +1,27 @@
// Copyright 2014 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Regression test for #24085. Errors were occuring in region
// inference due to the requirement that `'a:b'`, which was getting
// incorrectly translated in connection with the closure below.

#[derive(Copy,Clone)]
struct Path<'a:'b, 'b> {
x: &'a i32,
tail: Option<&'b Path<'a, 'b>>
}

#[allow(dead_code, unconditional_recursion)]
fn foo<'a,'b,F>(p: Path<'a, 'b>, mut f: F)
where F: for<'c> FnMut(Path<'a, 'c>) {
foo(p, |x| f(x))
}

fn main() { }

0 comments on commit 11720a1

Please sign in to comment.