From 75012f45cc805a4dca7211d296ecb890cad4db08 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 30 Aug 2017 16:06:05 -0400 Subject: [PATCH 1/2] make LUB/GLB of higher-ranked things actually do EQ --- src/librustc/infer/glb.rs | 5 +- src/librustc/infer/higher_ranked/mod.rs | 255 ------------------------ src/librustc/infer/lub.rs | 9 +- 3 files changed, 10 insertions(+), 259 deletions(-) diff --git a/src/librustc/infer/glb.rs b/src/librustc/infer/glb.rs index d7afeba7dc96b..e40548ac69972 100644 --- a/src/librustc/infer/glb.rs +++ b/src/librustc/infer/glb.rs @@ -74,7 +74,10 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx> -> RelateResult<'tcx, ty::Binder> where T: Relate<'tcx> { - self.fields.higher_ranked_glb(a, b, self.a_is_expected) + // Otherwise, we make the (overly strict) requirement that + // the two sides are equal. + self.relate_with_variance(ty::Variance::Invariant, a, b)?; + Ok(a.clone()) } } diff --git a/src/librustc/infer/higher_ranked/mod.rs b/src/librustc/infer/higher_ranked/mod.rs index 0d02420457e6b..9c95a330ec1da 100644 --- a/src/librustc/infer/higher_ranked/mod.rs +++ b/src/librustc/infer/higher_ranked/mod.rs @@ -195,261 +195,6 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> { Ok(HrMatchResult { value: a_value }) }); } - - pub fn higher_ranked_lub(&mut self, a: &Binder, b: &Binder, a_is_expected: bool) - -> RelateResult<'tcx, Binder> - where T: Relate<'tcx> - { - // Start a snapshot so we can examine "all bindings that were - // created as part of this type comparison". - return self.infcx.commit_if_ok(|snapshot| { - // Instantiate each bound region with a fresh region variable. - let span = self.trace.cause.span; - let (a_with_fresh, a_map) = - self.infcx.replace_late_bound_regions_with_fresh_var( - span, HigherRankedType, a); - let (b_with_fresh, _) = - self.infcx.replace_late_bound_regions_with_fresh_var( - span, HigherRankedType, b); - - // Collect constraints. - let result0 = - self.lub(a_is_expected).relate(&a_with_fresh, &b_with_fresh)?; - let result0 = - self.infcx.resolve_type_vars_if_possible(&result0); - debug!("lub result0 = {:?}", result0); - - // Generalize the regions appearing in result0 if possible - let new_vars = self.infcx.region_vars_confined_to_snapshot(snapshot); - let span = self.trace.cause.span; - let result1 = - fold_regions_in( - self.tcx(), - &result0, - |r, debruijn| generalize_region(self.infcx, span, snapshot, debruijn, - &new_vars, &a_map, r)); - - debug!("lub({:?},{:?}) = {:?}", - a, - b, - result1); - - Ok(ty::Binder(result1)) - }); - - fn generalize_region<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, - span: Span, - snapshot: &CombinedSnapshot, - debruijn: ty::DebruijnIndex, - new_vars: &[ty::RegionVid], - a_map: &FxHashMap>, - r0: ty::Region<'tcx>) - -> ty::Region<'tcx> { - // Regions that pre-dated the LUB computation stay as they are. - if !is_var_in_set(new_vars, r0) { - assert!(!r0.is_late_bound()); - debug!("generalize_region(r0={:?}): not new variable", r0); - return r0; - } - - let tainted = infcx.tainted_regions(snapshot, r0, TaintDirections::both()); - - // Variables created during LUB computation which are - // *related* to regions that pre-date the LUB computation - // stay as they are. - if !tainted.iter().all(|&r| is_var_in_set(new_vars, r)) { - debug!("generalize_region(r0={:?}): \ - non-new-variables found in {:?}", - r0, tainted); - assert!(!r0.is_late_bound()); - return r0; - } - - // Otherwise, the variable must be associated with at - // least one of the variables representing bound regions - // in both A and B. Replace the variable with the "first" - // bound region from A that we find it to be associated - // with. - for (a_br, a_r) in a_map { - if tainted.iter().any(|x| x == a_r) { - debug!("generalize_region(r0={:?}): \ - replacing with {:?}, tainted={:?}", - r0, *a_br, tainted); - return infcx.tcx.mk_region(ty::ReLateBound(debruijn, *a_br)); - } - } - - span_bug!( - span, - "region {:?} is not associated with any bound region from A!", - r0) - } - } - - pub fn higher_ranked_glb(&mut self, a: &Binder, b: &Binder, a_is_expected: bool) - -> RelateResult<'tcx, Binder> - where T: Relate<'tcx> - { - debug!("higher_ranked_glb({:?}, {:?})", - a, b); - - // Make a snapshot so we can examine "all bindings that were - // created as part of this type comparison". - return self.infcx.commit_if_ok(|snapshot| { - // Instantiate each bound region with a fresh region variable. - let (a_with_fresh, a_map) = - self.infcx.replace_late_bound_regions_with_fresh_var( - self.trace.cause.span, HigherRankedType, a); - let (b_with_fresh, b_map) = - self.infcx.replace_late_bound_regions_with_fresh_var( - self.trace.cause.span, HigherRankedType, b); - let a_vars = var_ids(self, &a_map); - let b_vars = var_ids(self, &b_map); - - // Collect constraints. - let result0 = - self.glb(a_is_expected).relate(&a_with_fresh, &b_with_fresh)?; - let result0 = - self.infcx.resolve_type_vars_if_possible(&result0); - debug!("glb result0 = {:?}", result0); - - // Generalize the regions appearing in result0 if possible - let new_vars = self.infcx.region_vars_confined_to_snapshot(snapshot); - let span = self.trace.cause.span; - let result1 = - fold_regions_in( - self.tcx(), - &result0, - |r, debruijn| generalize_region(self.infcx, span, snapshot, debruijn, - &new_vars, - &a_map, &a_vars, &b_vars, - r)); - - debug!("glb({:?},{:?}) = {:?}", - a, - b, - result1); - - Ok(ty::Binder(result1)) - }); - - fn generalize_region<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, - span: Span, - snapshot: &CombinedSnapshot, - debruijn: ty::DebruijnIndex, - new_vars: &[ty::RegionVid], - a_map: &FxHashMap>, - a_vars: &[ty::RegionVid], - b_vars: &[ty::RegionVid], - r0: ty::Region<'tcx>) - -> ty::Region<'tcx> { - if !is_var_in_set(new_vars, r0) { - assert!(!r0.is_late_bound()); - return r0; - } - - let tainted = infcx.tainted_regions(snapshot, r0, TaintDirections::both()); - - let mut a_r = None; - let mut b_r = None; - let mut only_new_vars = true; - for r in &tainted { - if is_var_in_set(a_vars, *r) { - if a_r.is_some() { - return fresh_bound_variable(infcx, debruijn); - } else { - a_r = Some(*r); - } - } else if is_var_in_set(b_vars, *r) { - if b_r.is_some() { - return fresh_bound_variable(infcx, debruijn); - } else { - b_r = Some(*r); - } - } else if !is_var_in_set(new_vars, *r) { - only_new_vars = false; - } - } - - // NB---I do not believe this algorithm computes - // (necessarily) the GLB. As written it can - // spuriously fail. In particular, if there is a case - // like: |fn(&a)| and fn(fn(&b)), where a and b are - // free, it will return fn(&c) where c = GLB(a,b). If - // however this GLB is not defined, then the result is - // an error, even though something like - // "fn(fn(&X))" where X is bound would be a - // subtype of both of those. - // - // The problem is that if we were to return a bound - // variable, we'd be computing a lower-bound, but not - // necessarily the *greatest* lower-bound. - // - // Unfortunately, this problem is non-trivial to solve, - // because we do not know at the time of computing the GLB - // whether a GLB(a,b) exists or not, because we haven't - // run region inference (or indeed, even fully computed - // the region hierarchy!). The current algorithm seems to - // works ok in practice. - - if a_r.is_some() && b_r.is_some() && only_new_vars { - // Related to exactly one bound variable from each fn: - return rev_lookup(infcx, span, a_map, a_r.unwrap()); - } else if a_r.is_none() && b_r.is_none() { - // Not related to bound variables from either fn: - assert!(!r0.is_late_bound()); - return r0; - } else { - // Other: - return fresh_bound_variable(infcx, debruijn); - } - } - - fn rev_lookup<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, - span: Span, - a_map: &FxHashMap>, - r: ty::Region<'tcx>) -> ty::Region<'tcx> - { - for (a_br, a_r) in a_map { - if *a_r == r { - return infcx.tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(1), *a_br)); - } - } - span_bug!( - span, - "could not find original bound region for {:?}", - r); - } - - fn fresh_bound_variable<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, - debruijn: ty::DebruijnIndex) - -> ty::Region<'tcx> { - infcx.region_vars.new_bound(debruijn) - } - } -} - -fn var_ids<'a, 'gcx, 'tcx>(fields: &CombineFields<'a, 'gcx, 'tcx>, - map: &FxHashMap>) - -> Vec { - map.iter() - .map(|(_, &r)| match *r { - ty::ReVar(r) => { r } - _ => { - span_bug!( - fields.trace.cause.span, - "found non-region-vid: {:?}", - r); - } - }) - .collect() -} - -fn is_var_in_set(new_vars: &[ty::RegionVid], r: ty::Region) -> bool { - match *r { - ty::ReVar(ref v) => new_vars.iter().any(|x| x == v), - _ => false - } } fn fold_regions_in<'a, 'gcx, 'tcx, T, F>(tcx: TyCtxt<'a, 'gcx, 'tcx>, diff --git a/src/librustc/infer/lub.rs b/src/librustc/infer/lub.rs index 04b470b29fc5e..31f88f833f456 100644 --- a/src/librustc/infer/lub.rs +++ b/src/librustc/infer/lub.rs @@ -8,10 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use super::combine::CombineFields; use super::InferCtxt; -use super::lattice::{self, LatticeDir}; use super::Subtype; +use super::combine::CombineFields; +use super::lattice::{self, LatticeDir}; use traits::ObligationCause; use ty::{self, Ty, TyCtxt}; @@ -74,7 +74,10 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx> -> RelateResult<'tcx, ty::Binder> where T: Relate<'tcx> { - self.fields.higher_ranked_lub(a, b, self.a_is_expected) + // Otherwise, we make the (overly strict) requirement that + // the two sides are equal. + self.relate_with_variance(ty::Variance::Invariant, a, b)?; + Ok(a.clone()) } } From 083c433caa48f75bfd682c4e631567a6591b18cd Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 9 Oct 2017 16:36:44 -0400 Subject: [PATCH 2/2] Revert "make LUB/GLB of higher-ranked things actually do EQ" This reverts commit 7226cdaca04fc51cfc37c2614b929ebfb4a31d6e. --- src/librustc/infer/glb.rs | 5 +- src/librustc/infer/higher_ranked/mod.rs | 255 ++++++++++++++++++++++++ src/librustc/infer/lub.rs | 9 +- 3 files changed, 259 insertions(+), 10 deletions(-) diff --git a/src/librustc/infer/glb.rs b/src/librustc/infer/glb.rs index e40548ac69972..d7afeba7dc96b 100644 --- a/src/librustc/infer/glb.rs +++ b/src/librustc/infer/glb.rs @@ -74,10 +74,7 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx> -> RelateResult<'tcx, ty::Binder> where T: Relate<'tcx> { - // Otherwise, we make the (overly strict) requirement that - // the two sides are equal. - self.relate_with_variance(ty::Variance::Invariant, a, b)?; - Ok(a.clone()) + self.fields.higher_ranked_glb(a, b, self.a_is_expected) } } diff --git a/src/librustc/infer/higher_ranked/mod.rs b/src/librustc/infer/higher_ranked/mod.rs index 9c95a330ec1da..0d02420457e6b 100644 --- a/src/librustc/infer/higher_ranked/mod.rs +++ b/src/librustc/infer/higher_ranked/mod.rs @@ -195,6 +195,261 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> { Ok(HrMatchResult { value: a_value }) }); } + + pub fn higher_ranked_lub(&mut self, a: &Binder, b: &Binder, a_is_expected: bool) + -> RelateResult<'tcx, Binder> + where T: Relate<'tcx> + { + // Start a snapshot so we can examine "all bindings that were + // created as part of this type comparison". + return self.infcx.commit_if_ok(|snapshot| { + // Instantiate each bound region with a fresh region variable. + let span = self.trace.cause.span; + let (a_with_fresh, a_map) = + self.infcx.replace_late_bound_regions_with_fresh_var( + span, HigherRankedType, a); + let (b_with_fresh, _) = + self.infcx.replace_late_bound_regions_with_fresh_var( + span, HigherRankedType, b); + + // Collect constraints. + let result0 = + self.lub(a_is_expected).relate(&a_with_fresh, &b_with_fresh)?; + let result0 = + self.infcx.resolve_type_vars_if_possible(&result0); + debug!("lub result0 = {:?}", result0); + + // Generalize the regions appearing in result0 if possible + let new_vars = self.infcx.region_vars_confined_to_snapshot(snapshot); + let span = self.trace.cause.span; + let result1 = + fold_regions_in( + self.tcx(), + &result0, + |r, debruijn| generalize_region(self.infcx, span, snapshot, debruijn, + &new_vars, &a_map, r)); + + debug!("lub({:?},{:?}) = {:?}", + a, + b, + result1); + + Ok(ty::Binder(result1)) + }); + + fn generalize_region<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, + span: Span, + snapshot: &CombinedSnapshot, + debruijn: ty::DebruijnIndex, + new_vars: &[ty::RegionVid], + a_map: &FxHashMap>, + r0: ty::Region<'tcx>) + -> ty::Region<'tcx> { + // Regions that pre-dated the LUB computation stay as they are. + if !is_var_in_set(new_vars, r0) { + assert!(!r0.is_late_bound()); + debug!("generalize_region(r0={:?}): not new variable", r0); + return r0; + } + + let tainted = infcx.tainted_regions(snapshot, r0, TaintDirections::both()); + + // Variables created during LUB computation which are + // *related* to regions that pre-date the LUB computation + // stay as they are. + if !tainted.iter().all(|&r| is_var_in_set(new_vars, r)) { + debug!("generalize_region(r0={:?}): \ + non-new-variables found in {:?}", + r0, tainted); + assert!(!r0.is_late_bound()); + return r0; + } + + // Otherwise, the variable must be associated with at + // least one of the variables representing bound regions + // in both A and B. Replace the variable with the "first" + // bound region from A that we find it to be associated + // with. + for (a_br, a_r) in a_map { + if tainted.iter().any(|x| x == a_r) { + debug!("generalize_region(r0={:?}): \ + replacing with {:?}, tainted={:?}", + r0, *a_br, tainted); + return infcx.tcx.mk_region(ty::ReLateBound(debruijn, *a_br)); + } + } + + span_bug!( + span, + "region {:?} is not associated with any bound region from A!", + r0) + } + } + + pub fn higher_ranked_glb(&mut self, a: &Binder, b: &Binder, a_is_expected: bool) + -> RelateResult<'tcx, Binder> + where T: Relate<'tcx> + { + debug!("higher_ranked_glb({:?}, {:?})", + a, b); + + // Make a snapshot so we can examine "all bindings that were + // created as part of this type comparison". + return self.infcx.commit_if_ok(|snapshot| { + // Instantiate each bound region with a fresh region variable. + let (a_with_fresh, a_map) = + self.infcx.replace_late_bound_regions_with_fresh_var( + self.trace.cause.span, HigherRankedType, a); + let (b_with_fresh, b_map) = + self.infcx.replace_late_bound_regions_with_fresh_var( + self.trace.cause.span, HigherRankedType, b); + let a_vars = var_ids(self, &a_map); + let b_vars = var_ids(self, &b_map); + + // Collect constraints. + let result0 = + self.glb(a_is_expected).relate(&a_with_fresh, &b_with_fresh)?; + let result0 = + self.infcx.resolve_type_vars_if_possible(&result0); + debug!("glb result0 = {:?}", result0); + + // Generalize the regions appearing in result0 if possible + let new_vars = self.infcx.region_vars_confined_to_snapshot(snapshot); + let span = self.trace.cause.span; + let result1 = + fold_regions_in( + self.tcx(), + &result0, + |r, debruijn| generalize_region(self.infcx, span, snapshot, debruijn, + &new_vars, + &a_map, &a_vars, &b_vars, + r)); + + debug!("glb({:?},{:?}) = {:?}", + a, + b, + result1); + + Ok(ty::Binder(result1)) + }); + + fn generalize_region<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, + span: Span, + snapshot: &CombinedSnapshot, + debruijn: ty::DebruijnIndex, + new_vars: &[ty::RegionVid], + a_map: &FxHashMap>, + a_vars: &[ty::RegionVid], + b_vars: &[ty::RegionVid], + r0: ty::Region<'tcx>) + -> ty::Region<'tcx> { + if !is_var_in_set(new_vars, r0) { + assert!(!r0.is_late_bound()); + return r0; + } + + let tainted = infcx.tainted_regions(snapshot, r0, TaintDirections::both()); + + let mut a_r = None; + let mut b_r = None; + let mut only_new_vars = true; + for r in &tainted { + if is_var_in_set(a_vars, *r) { + if a_r.is_some() { + return fresh_bound_variable(infcx, debruijn); + } else { + a_r = Some(*r); + } + } else if is_var_in_set(b_vars, *r) { + if b_r.is_some() { + return fresh_bound_variable(infcx, debruijn); + } else { + b_r = Some(*r); + } + } else if !is_var_in_set(new_vars, *r) { + only_new_vars = false; + } + } + + // NB---I do not believe this algorithm computes + // (necessarily) the GLB. As written it can + // spuriously fail. In particular, if there is a case + // like: |fn(&a)| and fn(fn(&b)), where a and b are + // free, it will return fn(&c) where c = GLB(a,b). If + // however this GLB is not defined, then the result is + // an error, even though something like + // "fn(fn(&X))" where X is bound would be a + // subtype of both of those. + // + // The problem is that if we were to return a bound + // variable, we'd be computing a lower-bound, but not + // necessarily the *greatest* lower-bound. + // + // Unfortunately, this problem is non-trivial to solve, + // because we do not know at the time of computing the GLB + // whether a GLB(a,b) exists or not, because we haven't + // run region inference (or indeed, even fully computed + // the region hierarchy!). The current algorithm seems to + // works ok in practice. + + if a_r.is_some() && b_r.is_some() && only_new_vars { + // Related to exactly one bound variable from each fn: + return rev_lookup(infcx, span, a_map, a_r.unwrap()); + } else if a_r.is_none() && b_r.is_none() { + // Not related to bound variables from either fn: + assert!(!r0.is_late_bound()); + return r0; + } else { + // Other: + return fresh_bound_variable(infcx, debruijn); + } + } + + fn rev_lookup<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, + span: Span, + a_map: &FxHashMap>, + r: ty::Region<'tcx>) -> ty::Region<'tcx> + { + for (a_br, a_r) in a_map { + if *a_r == r { + return infcx.tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(1), *a_br)); + } + } + span_bug!( + span, + "could not find original bound region for {:?}", + r); + } + + fn fresh_bound_variable<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, + debruijn: ty::DebruijnIndex) + -> ty::Region<'tcx> { + infcx.region_vars.new_bound(debruijn) + } + } +} + +fn var_ids<'a, 'gcx, 'tcx>(fields: &CombineFields<'a, 'gcx, 'tcx>, + map: &FxHashMap>) + -> Vec { + map.iter() + .map(|(_, &r)| match *r { + ty::ReVar(r) => { r } + _ => { + span_bug!( + fields.trace.cause.span, + "found non-region-vid: {:?}", + r); + } + }) + .collect() +} + +fn is_var_in_set(new_vars: &[ty::RegionVid], r: ty::Region) -> bool { + match *r { + ty::ReVar(ref v) => new_vars.iter().any(|x| x == v), + _ => false + } } fn fold_regions_in<'a, 'gcx, 'tcx, T, F>(tcx: TyCtxt<'a, 'gcx, 'tcx>, diff --git a/src/librustc/infer/lub.rs b/src/librustc/infer/lub.rs index 31f88f833f456..04b470b29fc5e 100644 --- a/src/librustc/infer/lub.rs +++ b/src/librustc/infer/lub.rs @@ -8,10 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use super::InferCtxt; -use super::Subtype; use super::combine::CombineFields; +use super::InferCtxt; use super::lattice::{self, LatticeDir}; +use super::Subtype; use traits::ObligationCause; use ty::{self, Ty, TyCtxt}; @@ -74,10 +74,7 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx> -> RelateResult<'tcx, ty::Binder> where T: Relate<'tcx> { - // Otherwise, we make the (overly strict) requirement that - // the two sides are equal. - self.relate_with_variance(ty::Variance::Invariant, a, b)?; - Ok(a.clone()) + self.fields.higher_ranked_lub(a, b, self.a_is_expected) } }