From 11720a1964721008a062b413aa133959304e6d3e Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 17 Apr 2015 10:01:45 -0400 Subject: [PATCH] Expand the "givens" set to cover transitive relations. The givens array 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 #24085. --- src/librustc/middle/cfg/mod.rs | 3 +- src/librustc/middle/graph.rs | 6 +-- .../middle/infer/region_inference/mod.rs | 43 ++++++++++++++----- src/librustc_trans/trans/base.rs | 3 +- src/test/run-pass/issue-24085.rs | 27 ++++++++++++ 5 files changed, 67 insertions(+), 15 deletions(-) create mode 100644 src/test/run-pass/issue-24085.rs diff --git a/src/librustc/middle/cfg/mod.rs b/src/librustc/middle/cfg/mod.rs index ad4fdcd7b834e..1ca6438c1ea3e 100644 --- a/src/librustc/middle/cfg/mod.rs +++ b/src/librustc/middle/cfg/mod.rs @@ -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) } } diff --git a/src/librustc/middle/graph.rs b/src/librustc/middle/graph.rs index a9ac61b49eca8..34b158334d97e 100644 --- a/src/librustc/middle/graph.rs +++ b/src/librustc/middle/graph.rs @@ -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 { while let Some(idx) = self.stack.pop() { if !self.visited.insert(idx.node_id()) { continue; @@ -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; diff --git a/src/librustc/middle/infer/region_inference/mod.rs b/src/librustc/middle/infer/region_inference/mod.rs index d41fdc5f09acd..4a186f788332b 100644 --- a/src/librustc/middle/infer/region_inference/mod.rs +++ b/src/librustc/middle/infer/region_inference/mod.rs @@ -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 @@ -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={}", @@ -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>) -> Vec { @@ -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(_) => { @@ -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 => { diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 7c51a193b984b..daefd854dfe61 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -1110,7 +1110,8 @@ fn build_cfg(tcx: &ty::ctxt, id: ast::NodeId) -> (ast::NodeId, Option) // 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 { diff --git a/src/test/run-pass/issue-24085.rs b/src/test/run-pass/issue-24085.rs new file mode 100644 index 0000000000000..3617b0d3a5777 --- /dev/null +++ b/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 or the MIT license +// , 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() { }