From bdc1bfd8f1740461d2ee39ba41f5245ed8913a2a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 7 Jan 2015 09:36:11 -0500 Subject: [PATCH 1/5] Rename common::normalize to common::erase_regions --- src/librustc/middle/ty_fold.rs | 5 ++++- src/librustc_trans/trans/callee.rs | 2 +- src/librustc_trans/trans/closure.rs | 2 +- src/librustc_trans/trans/common.rs | 22 +++++++++++++--------- src/librustc_trans/trans/type_of.rs | 2 +- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/librustc/middle/ty_fold.rs b/src/librustc/middle/ty_fold.rs index 424e357d8d456..e18e62948fb66 100644 --- a/src/librustc/middle/ty_fold.rs +++ b/src/librustc/middle/ty_fold.rs @@ -853,7 +853,10 @@ impl<'a, 'tcx> TypeFolder<'tcx> for RegionFolder<'a, 'tcx> /////////////////////////////////////////////////////////////////////////// // Region eraser // -// Replaces all free regions with 'static. Useful in trans. +// Replaces all free regions with 'static. Useful in contexts, such as +// method probing, where precise region relationships are not +// important. Note that in trans you should use +// `common::erase_regions` instead. pub struct RegionEraser<'a, 'tcx: 'a> { tcx: &'a ty::ctxt<'tcx>, diff --git a/src/librustc_trans/trans/callee.rs b/src/librustc_trans/trans/callee.rs index b7b486f1d0a52..4cfd831a3251b 100644 --- a/src/librustc_trans/trans/callee.rs +++ b/src/librustc_trans/trans/callee.rs @@ -265,7 +265,7 @@ pub fn trans_fn_pointer_shim<'a, 'tcx>( let _icx = push_ctxt("trans_fn_pointer_shim"); let tcx = ccx.tcx(); - let bare_fn_ty = normalize_ty(tcx, bare_fn_ty); + let bare_fn_ty = erase_regions(tcx, &bare_fn_ty); match ccx.fn_pointer_shims().borrow().get(&bare_fn_ty) { Some(&llval) => { return llval; } None => { } diff --git a/src/librustc_trans/trans/closure.rs b/src/librustc_trans/trans/closure.rs index ad2ed67b22c9a..06a1182b1f432 100644 --- a/src/librustc_trans/trans/closure.rs +++ b/src/librustc_trans/trans/closure.rs @@ -466,7 +466,7 @@ pub fn get_or_create_declaration_if_unboxed_closure<'a, 'tcx>(ccx: &CrateContext // Normalize type so differences in regions and typedefs don't cause // duplicate declarations - let function_type = normalize_ty(ccx.tcx(), function_type); + let function_type = erase_regions(ccx.tcx(), &function_type); let params = match function_type.sty { ty::ty_unboxed_closure(_, _, ref substs) => substs.types.clone(), _ => unreachable!() diff --git a/src/librustc_trans/trans/common.rs b/src/librustc_trans/trans/common.rs index 237fc1856369b..bdec9ad378b2b 100644 --- a/src/librustc_trans/trans/common.rs +++ b/src/librustc_trans/trans/common.rs @@ -58,16 +58,21 @@ use util::nodemap::FnvHashSet; pub use trans::context::CrateContext; -/// Returns an equivalent type with all the typedefs and self regions removed. -pub fn normalize_ty<'tcx>(cx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> Ty<'tcx> { - let u = TypeNormalizer(cx).fold_ty(ty); - debug!("normalize_ty({}) = {}", - ty.repr(cx), u.repr(cx)); - return u; +/// Returns an equivalent value with all free regions removed (note +/// that late-bound regions remain, because they are important for +/// subtyping, but they are anonymized and normalized as well). This +/// is a stronger, caching version of `ty_fold::erase_regions`. +pub fn erase_regions<'tcx,T>(cx: &ty::ctxt<'tcx>, value: &T) -> T + where T : TypeFoldable<'tcx> + Repr<'tcx> +{ + let value1 = value.fold_with(&mut RegionEraser(cx)); + debug!("erase_regions({}) = {}", + value.repr(cx), value1.repr(cx)); + return value1; - struct TypeNormalizer<'a, 'tcx: 'a>(&'a ty::ctxt<'tcx>); + struct RegionEraser<'a, 'tcx: 'a>(&'a ty::ctxt<'tcx>); - impl<'a, 'tcx> TypeFolder<'tcx> for TypeNormalizer<'a, 'tcx> { + impl<'a, 'tcx> TypeFolder<'tcx> for RegionEraser<'a, 'tcx> { fn tcx(&self) -> &ty::ctxt<'tcx> { self.0 } fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> { @@ -84,7 +89,6 @@ pub fn normalize_ty<'tcx>(cx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> Ty<'tcx> { fn fold_binder(&mut self, t: &ty::Binder) -> ty::Binder where T : TypeFoldable<'tcx> + Repr<'tcx> { - // FIXME(#20526) this should replace `enter_region_binder`/`exit_region_binder`. let u = ty::anonymize_late_bound_regions(self.tcx(), t); ty_fold::super_fold_binder(self, &u) } diff --git a/src/librustc_trans/trans/type_of.rs b/src/librustc_trans/trans/type_of.rs index 19d50cdd48320..ef5394f06b3b8 100644 --- a/src/librustc_trans/trans/type_of.rs +++ b/src/librustc_trans/trans/type_of.rs @@ -285,7 +285,7 @@ pub fn type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>) -> Type { // Rust types are defined as the same LLVM types. If we don't do // this then, e.g. `Option<{myfield: bool}>` would be a different // type than `Option`. - let t_norm = normalize_ty(cx.tcx(), t); + let t_norm = erase_regions(cx.tcx(), &t); if t != t_norm { let llty = type_of(cx, t_norm); From cf136cd3507a5e929cff8d03179c0f6d96bdb687 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 7 Jan 2015 14:04:37 -0500 Subject: [PATCH 2/5] Use the `erase_regions` helper within trans in deference to `ty_fold::erase_regions`; also erase regions whenever we normalize associated types. --- src/librustc_trans/trans/common.rs | 2 +- src/librustc_trans/trans/monomorphize.rs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc_trans/trans/common.rs b/src/librustc_trans/trans/common.rs index bdec9ad378b2b..017593f387e23 100644 --- a/src/librustc_trans/trans/common.rs +++ b/src/librustc_trans/trans/common.rs @@ -992,7 +992,7 @@ pub fn fulfill_obligation<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, let tcx = ccx.tcx(); // Remove any references to regions; this helps improve caching. -let trait_ref = ty_fold::erase_regions(tcx, trait_ref); +let trait_ref = erase_regions(tcx, &trait_ref); // First check the cache. match ccx.trait_cache().borrow().get(&trait_ref) { diff --git a/src/librustc_trans/trans/monomorphize.rs b/src/librustc_trans/trans/monomorphize.rs index e2594765f4fda..a35bae07da09a 100644 --- a/src/librustc_trans/trans/monomorphize.rs +++ b/src/librustc_trans/trans/monomorphize.rs @@ -315,8 +315,10 @@ pub fn normalize_associated_type<'tcx,T>(tcx: &ty::ctxt<'tcx>, value: &T) -> T { debug!("normalize_associated_type(t={})", value.repr(tcx)); + let value = erase_regions(tcx, value); + if !value.has_projection_types() { - return value.clone(); + return value; } // FIXME(#20304) -- cache @@ -326,7 +328,7 @@ pub fn normalize_associated_type<'tcx,T>(tcx: &ty::ctxt<'tcx>, value: &T) -> T let mut selcx = traits::SelectionContext::new(&infcx, &typer); let cause = traits::ObligationCause::dummy(); let traits::Normalized { value: result, obligations } = - traits::normalize(&mut selcx, cause, value); + traits::normalize(&mut selcx, cause, &value); debug!("normalize_associated_type: result={} obligations={}", result.repr(tcx), From aec62af7427083ac7f6dcb2fd484876199147fdc Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 7 Jan 2015 14:05:32 -0500 Subject: [PATCH 3/5] Solve rather subtle bug in `replace_late_bound_regions` -- we were passing the debruijn index in so that callees could construct late-bound regions at the right depth, but then the result was cached. When the cached result was used, it might be at the wrong depth. So now we don't pass the result in and instead simply adjust the depth to match the current nesting level as we go. --- .../middle/infer/higher_ranked/mod.rs | 2 +- src/librustc/middle/infer/mod.rs | 2 +- src/librustc/middle/ty.rs | 27 +++++++++++++------ src/librustc/util/ppaux.rs | 4 +-- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/librustc/middle/infer/higher_ranked/mod.rs b/src/librustc/middle/infer/higher_ranked/mod.rs index 073052dd36870..40f32a51e22f7 100644 --- a/src/librustc/middle/infer/higher_ranked/mod.rs +++ b/src/librustc/middle/infer/higher_ranked/mod.rs @@ -468,7 +468,7 @@ pub fn skolemize_late_bound_regions<'a,'tcx,T>(infcx: &InferCtxt<'a,'tcx>, * when higher-ranked things are involved. See `doc.rs` for more details. */ - let (result, map) = ty::replace_late_bound_regions(infcx.tcx, binder, |br, _| { + let (result, map) = ty::replace_late_bound_regions(infcx.tcx, binder, |br| { infcx.region_vars.new_skolemized(br, &snapshot.region_vars_snapshot) }); diff --git a/src/librustc/middle/infer/mod.rs b/src/librustc/middle/infer/mod.rs index 3f18af3d768e4..c1f0c10e11fe9 100644 --- a/src/librustc/middle/infer/mod.rs +++ b/src/librustc/middle/infer/mod.rs @@ -1056,7 +1056,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ty::replace_late_bound_regions( self.tcx, value, - |br, _| self.next_region_var(LateBoundRegion(span, br, lbrct))) + |br| self.next_region_var(LateBoundRegion(span, br, lbrct))) } /// See `verify_generic_bound` method in `region_inference` diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 9b9a58178cc34..2c9ff35364b20 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -6672,7 +6672,7 @@ pub fn liberate_late_bound_regions<'tcx, T>( { replace_late_bound_regions( tcx, value, - |br, _| ty::ReFree(ty::FreeRegion{scope: scope, bound_region: br})).0 + |br| ty::ReFree(ty::FreeRegion{scope: scope, bound_region: br})).0 } pub fn count_late_bound_regions<'tcx, T>( @@ -6681,7 +6681,7 @@ pub fn count_late_bound_regions<'tcx, T>( -> uint where T : TypeFoldable<'tcx> + Repr<'tcx> { - let (_, skol_map) = replace_late_bound_regions(tcx, value, |_, _| ty::ReStatic); + let (_, skol_map) = replace_late_bound_regions(tcx, value, |_| ty::ReStatic); skol_map.len() } @@ -6712,7 +6712,7 @@ pub fn erase_late_bound_regions<'tcx, T>( -> T where T : TypeFoldable<'tcx> + Repr<'tcx> { - replace_late_bound_regions(tcx, value, |_, _| ty::ReStatic).0 + replace_late_bound_regions(tcx, value, |_| ty::ReStatic).0 } /// Rewrite any late-bound regions so that they are anonymous. Region numbers are @@ -6730,9 +6730,9 @@ pub fn anonymize_late_bound_regions<'tcx, T>( where T : TypeFoldable<'tcx> + Repr<'tcx>, { let mut counter = 0; - ty::Binder(replace_late_bound_regions(tcx, sig, |_, db| { + ty::Binder(replace_late_bound_regions(tcx, sig, |_| { counter += 1; - ReLateBound(db, BrAnon(counter)) + ReLateBound(ty::DebruijnIndex::new(1), BrAnon(counter)) }).0) } @@ -6743,7 +6743,7 @@ pub fn replace_late_bound_regions<'tcx, T, F>( mut mapf: F) -> (T, FnvHashMap) where T : TypeFoldable<'tcx> + Repr<'tcx>, - F : FnMut(BoundRegion, DebruijnIndex) -> ty::Region, + F : FnMut(BoundRegion) -> ty::Region, { debug!("replace_late_bound_regions({})", binder.repr(tcx)); @@ -6755,8 +6755,19 @@ pub fn replace_late_bound_regions<'tcx, T, F>( debug!("region={}", region.repr(tcx)); match region { ty::ReLateBound(debruijn, br) if debruijn.depth == current_depth => { - * map.entry(br).get().unwrap_or_else( - |vacant_entry| vacant_entry.insert(mapf(br, debruijn))) + let region = + * map.entry(br).get().unwrap_or_else( + |vacant_entry| vacant_entry.insert(mapf(br))); + + if let ty::ReLateBound(debruijn1, br) = region { + // If the callback returns a late-bound region, + // that region should always use depth 1. Then we + // adjust it to the correct depth. + assert_eq!(debruijn1.depth, 1); + ty::ReLateBound(debruijn, br) + } else { + region + } } _ => { region diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index 2d433369366eb..0cf676b7035b4 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -1182,8 +1182,8 @@ impl<'tcx, T> UserString<'tcx> for ty::Binder // the output. We'll probably want to tweak this over time to // decide just how much information to give. let mut names = Vec::new(); - let (unbound_value, _) = ty::replace_late_bound_regions(tcx, self, |br, debruijn| { - ty::ReLateBound(debruijn, match br { + let (unbound_value, _) = ty::replace_late_bound_regions(tcx, self, |br| { + ty::ReLateBound(ty::DebruijnIndex::new(1), match br { ty::BrNamed(_, name) => { names.push(token::get_name(name)); br From 448ddad87768a8ecef4dc8fc1394fdebbc487f85 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 7 Jan 2015 14:06:04 -0500 Subject: [PATCH 4/5] Better debug output in `decl_rust_fn`. The lack of output here has caused me quite a bit of hair-pulling. --- src/librustc_trans/trans/base.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 057d0f378e6f4..ceceabb80f57f 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -281,8 +281,15 @@ pub fn kind_for_unboxed_closure(ccx: &CrateContext, closure_id: ast::DefId) pub fn decl_rust_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, fn_ty: Ty<'tcx>, name: &str) -> ValueRef { + debug!("decl_rust_fn(fn_ty={}, name={:?})", + fn_ty.repr(ccx.tcx()), + name); + let fn_ty = monomorphize::normalize_associated_type(ccx.tcx(), &fn_ty); + debug!("decl_rust_fn: fn_ty={} (after normalized associated types)", + fn_ty.repr(ccx.tcx())); + let function_type; // placeholder so that the memory ownership works out ok let (sig, abi, env) = match fn_ty.sty { @@ -305,10 +312,12 @@ pub fn decl_rust_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, let sig = ty::erase_late_bound_regions(ccx.tcx(), sig); let sig = ty::Binder(sig); + debug!("decl_rust_fn: sig={} (after erasing regions)", + sig.repr(ccx.tcx())); + let llfty = type_of_rust_fn(ccx, env, &sig, abi); - debug!("decl_rust_fn(sig={}, type={})", - sig.repr(ccx.tcx()), + debug!("decl_rust_fn: llfty={}", ccx.tn().type_to_string(llfty)); let llfn = decl_fn(ccx, name, llvm::CCallConv, llfty, sig.0.output /* (1) */); From a70428aa09d0ffc2e9ecd35c22077cc07da6719c Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 7 Jan 2015 14:07:55 -0500 Subject: [PATCH 5/5] Add regression test for #20582. Fixes #20582. --- ...ciated-types-region-erasure-issue-20582.rs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 src/test/run-pass/associated-types-region-erasure-issue-20582.rs diff --git a/src/test/run-pass/associated-types-region-erasure-issue-20582.rs b/src/test/run-pass/associated-types-region-erasure-issue-20582.rs new file mode 100644 index 0000000000000..03ab8f7e43136 --- /dev/null +++ b/src/test/run-pass/associated-types-region-erasure-issue-20582.rs @@ -0,0 +1,27 @@ +// Copyright 2015 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 #20582. This test caused an ICE related to +// inconsistent region erasure in trans. + +struct Foo<'a> { + buf: &'a[u8] +} + +impl<'a> Iterator for Foo<'a> { + type Item = &'a[u8]; + + fn next(&mut self) -> Option<::Item> { + Some(self.buf) + } +} + +fn main() { +}