Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor DebruijnIndex to be 0-based #50475

Merged
merged 13 commits into from
May 29, 2018
Merged

Refactor DebruijnIndex to be 0-based #50475

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
69ab6f2
introduce `shifted_in`, `shifted_out` and friends
nikomatsakis May 25, 2018
9c5a450
port `fold_regions` and friends to use debruijn indices directly
nikomatsakis May 25, 2018
b5018de
make `shifted_in` and `shifted_out` const fns
nikomatsakis May 25, 2018
8bd4bff
stop invoking `DebruijnIndex::new` directly
nikomatsakis May 25, 2018
e2f7f4a
port `nice_region_error` code to not track depth but rather index
nikomatsakis May 25, 2018
f965b79
rewrite the hasher to not access `depth` field
nikomatsakis May 28, 2018
34c9ae7
refactor `resolve_lifetime` to track outer-index, not depth
nikomatsakis May 28, 2018
4aeb6ef
replace use of DebruijnIndex in `for_each_free_region`
nikomatsakis May 28, 2018
06b2a3f
convert `LateBoundRegionsCollector` to track a debruijn index
nikomatsakis May 28, 2018
8f15d1e
replace `binder_depth` in `LateBoundRegionsDetector`
nikomatsakis May 28, 2018
7e15e0b
remove use of depth from `TyS` and replace with a debruijn index
nikomatsakis May 28, 2018
22a25d9
make depth private
nikomatsakis May 28, 2018
783fe4f
change to 0-based indices
nikomatsakis May 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refactor resolve_lifetime to track outer-index, not depth 
Co-authored-by: csmoe <[email protected]>
  • Loading branch information
@nikomatsakis @csmoe
nikomatsakis and csmoe committed May 28, 2018
commit 34c9ae77f74eae4ff50bd10f422986d6da372013
26 changes: 11 additions & 15 deletions src/librustc/middle/resolve_lifetime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,15 @@ impl Region {
}
}

fn from_depth(self, depth: u32) -> Region {
fn shifted_out_to_binder(self, binder: ty::DebruijnIndex) -> Region {
match self {
Region::LateBound(debruijn, id, origin) => Region::LateBound(
ty::DebruijnIndex {
depth: debruijn.depth - (depth - 1),
},
debruijn.shifted_out_to_binder(binder),
id,
origin,
),
Region::LateBoundAnon(debruijn, index) => Region::LateBoundAnon(
ty::DebruijnIndex {
depth: debruijn.depth - (depth - 1),
},
debruijn.shifted_out_to_binder(binder),
index,
),
_ => self,
Expand Down Expand Up @@ -1858,7 +1854,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
.map(|(i, input)| {
let mut gather = GatherLifetimes {
map: self.map,
binder_depth: 1,
outer_index: ty::DebruijnIndex::INNERMOST,
have_bound_regions: false,
lifetimes: FxHashSet(),
};
Expand Down Expand Up @@ -1899,7 +1895,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {

struct GatherLifetimes<'a> {
map: &'a NamedRegionMap,
binder_depth: u32,
outer_index: ty::DebruijnIndex,
have_bound_regions: bool,
lifetimes: FxHashSet<Region>,
}
Expand All @@ -1911,7 +1907,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {

fn visit_ty(&mut self, ty: &hir::Ty) {
if let hir::TyBareFn(_) = ty.node {
self.binder_depth += 1;
self.outer_index.shift_in(1);
}
if let hir::TyTraitObject(ref bounds, ref lifetime) = ty.node {
for bound in bounds {
Expand All @@ -1927,7 +1923,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
intravisit::walk_ty(self, ty);
}
if let hir::TyBareFn(_) = ty.node {
self.binder_depth -= 1;
self.outer_index.shift_out(1);
}
}

Expand All @@ -1946,22 +1942,22 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
trait_ref: &hir::PolyTraitRef,
modifier: hir::TraitBoundModifier,
) {
self.binder_depth += 1;
self.outer_index.shift_in(1);
intravisit::walk_poly_trait_ref(self, trait_ref, modifier);
self.binder_depth -= 1;
self.outer_index.shift_out(1);
}

fn visit_lifetime(&mut self, lifetime_ref: &hir::Lifetime) {
if let Some(&lifetime) = self.map.defs.get(&lifetime_ref.id) {
match lifetime {
Region::LateBound(debruijn, _, _) | Region::LateBoundAnon(debruijn, _)
if debruijn.depth < self.binder_depth =>
if debruijn < self.outer_index =>
{
self.have_bound_regions = true;
}
_ => {
self.lifetimes
.insert(lifetime.from_depth(self.binder_depth));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I suspect this is where the bug is coming from. The binder_depth in the "before code" is equal to index, but (if you look at the definition of from_depth) you will see that used to be computing adjusting by depth - 1, basically.

I think the way I would want to compute the "depth" by giving two debruijn indices. In particular, we might do something like self.outer_index.depth_from(INNERMOST), where we have:

impl DebruijnIndex {
    /// Assuming that these two debruijn indices are both in scope,
    /// returns the number of binding levels *between* them.
    /// This is often used if `outer` represents some starting binding
    /// and `self` represents the current innermost binding; it would then
    /// compute the number of bindings passed over so far.
    ///
    /// As the name suggests, `outer` is assumed  to be the
    /// outermost depth of the two.
    fn depth_from(self, outer: DebruijnIndex) -> usize {
        assert!(self.index >= other.index); // or return the absolute value
        self.index - other.index
    }
}

.insert(lifetime.shifted_out_to_binder(self.outer_index));
}
}
}
Expand Down