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

Jump to bottom

[WIP] Make Debruijn indices zero-based #49840

Closed
wants to merge 2 commits into from
Closed

[WIP] Make Debruijn indices zero-based #49840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55a6c62
Make Debruijn indices zero-based
Apr 10, 2018
8cdfe8c
Adjust type Debruijn depth calculations
Apr 10, 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
2 changes: 1 addition & 1 deletion src/librustc/infer/higher_ranked/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, '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));
return infcx.tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(0), *a_br));
}
}
span_bug!(
Expand Down
10 changes: 5 additions & 5 deletions src/librustc/middle/resolve_lifetime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl Region {
}

fn late(hir_map: &Map, def: &hir::LifetimeDef) -> (hir::LifetimeName, Region) {
let depth = ty::DebruijnIndex::new(1);
let depth = ty::DebruijnIndex::new(0);
let def_id = hir_map.local_def_id(def.lifetime.id);
let origin = LifetimeDefOrigin::from_is_in_band(def.in_band);
(def.lifetime.name, Region::LateBound(depth, def_id, origin))
Expand All @@ -107,7 +107,7 @@ impl Region {
fn late_anon(index: &Cell<u32>) -> Region {
let i = index.get();
index.set(i + 1);
let depth = ty::DebruijnIndex::new(1);
let depth = ty::DebruijnIndex::new(0);
Region::LateBoundAnon(depth, i)
}

Expand Down Expand Up @@ -137,14 +137,14 @@ impl Region {
match self {
Region::LateBound(debruijn, id, origin) => Region::LateBound(
ty::DebruijnIndex {
Copy link
Contributor

Choose a reason for hiding this comment

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

pre-existing, but from_depth sure needs a comment =) let me try to remember what it's purpose is...

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, it seems like it is basically a variant of shifted, but it goes in the opposite direction. I think it should probably be renamed; chalk calls this shift_down, I think. In any case, a comment would be good:

Removes depth level of binders from this region. region must not be bound within those binders or else this will panic.

To that end, up/down are not especially intuitive names. Maybe something like shift_out or pull_out (from binders)? We would then rename shifted to shift_in or push_down (through binders)? Maybe remove_binders and in_binders?

Copy link
Author

@muscar muscar Apr 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'll have a look and rename these :D Thanks for the hints.

depth: debruijn.depth - (depth - 1),
depth: debruijn.depth - depth,
},
id,
origin,
),
Region::LateBoundAnon(debruijn, index) => Region::LateBoundAnon(
ty::DebruijnIndex {
depth: debruijn.depth - (depth - 1),
depth: debruijn.depth - depth,
},
index,
),
Expand Down Expand Up @@ -1781,7 +1781,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
.map(|(i, input)| {
let mut gather = GatherLifetimes {
map: self.map,
binder_depth: 1,
binder_depth: 0,
have_bound_regions: false,
lifetimes: FxHashSet(),
};
Expand Down
10 changes: 5 additions & 5 deletions src/librustc/ty/fold.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ impl<'a, 'gcx, 'tcx> RegionFolder<'a, 'gcx, 'tcx> {
RegionFolder {
tcx,
skipped_regions,
current_depth: 1,
current_depth: 0,
fld_r,
}
}
Expand Down Expand Up @@ -448,7 +448,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
let mut counter = 0;
Binder(self.replace_late_bound_regions(sig, |_| {
counter += 1;
self.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(1), ty::BrAnon(counter)))
self.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(0), ty::BrAnon(counter)))
}).0)
}
}
Expand All @@ -460,7 +460,7 @@ impl<'a, 'gcx, 'tcx> RegionReplacer<'a, 'gcx, 'tcx> {
{
RegionReplacer {
tcx,
current_depth: 1,
current_depth: 0,
fld_r,
map: BTreeMap::default()
}
Expand All @@ -478,7 +478,7 @@ impl<'a, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for RegionReplacer<'a, 'gcx, 'tcx> {
}

fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
if !t.has_regions_escaping_depth(self.current_depth-1) {
if !t.has_regions_escaping_depth(self.current_depth) {
return t;
}

Expand Down Expand Up @@ -635,7 +635,7 @@ struct LateBoundRegionsCollector {
impl LateBoundRegionsCollector {
fn new(just_constrained: bool) -> Self {
LateBoundRegionsCollector {
current_depth: 1,
current_depth: 0,
regions: FxHashSet(),
just_constrained,
}
Expand Down
5 changes: 2 additions & 3 deletions src/librustc/ty/sty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,6 @@ impl<'a, 'tcx, 'gcx> PolyExistentialProjection<'tcx> {

impl DebruijnIndex {
pub fn new(depth: u32) -> DebruijnIndex {
assert!(depth > 0);
Copy link
Member

@lnicola lnicola Apr 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Does DebruijnIndex::from_depth need to be changed to remove the shift by 1?

Copy link
Author

Choose a reason for hiding this comment

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

Good point. I'll give it a go locally.

DebruijnIndex { depth: depth }
}

Expand Down Expand Up @@ -1188,11 +1187,11 @@ impl RegionKind {
}
}

/// Returns the depth of `self` from the (1-based) binding level `depth`
/// Returns the depth of `self` from the (0-based) binding level `depth`
pub fn from_depth(&self, depth: u32) -> RegionKind {
match *self {
ty::ReLateBound(debruijn, r) => ty::ReLateBound(DebruijnIndex {
depth: debruijn.depth - (depth - 1)
depth: debruijn.depth - depth
}, r),
r => r
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
-> Option<ty::Binder<Ty<'tcx>>>
{
let closure_ty = self.mk_closure(closure_def_id, closure_substs);
let env_region = ty::ReLateBound(ty::DebruijnIndex::new(1), ty::BrEnv);
let env_region = ty::ReLateBound(ty::DebruijnIndex::new(0), ty::BrEnv);
let closure_kind_ty = closure_substs.closure_kind_ty(closure_def_id, self);
let closure_kind = closure_kind_ty.to_opt_closure_kind()?;
let env_ty = match closure_kind {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/util/ppaux.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ impl PrintContext {
name)
}
};
tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(1), br))
tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(0), br))
}).0;
start_or_continue(f, "", "> ")?;

Expand Down
14 changes: 7 additions & 7 deletions src/librustc_driver/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ impl<'a, 'gcx, 'tcx> Env<'a, 'gcx, 'tcx> {
}

pub fn t_rptr_late_bound(&self, id: u32) -> Ty<'tcx> {
let r = self.re_late_bound_with_debruijn(id, ty::DebruijnIndex::new(1));
let r = self.re_late_bound_with_debruijn(id, ty::DebruijnIndex::new(0));
self.infcx.tcx.mk_imm_ref(r, self.tcx().types.isize)
}

Expand Down Expand Up @@ -484,7 +484,7 @@ fn subst_ty_renumber_bound() {

// t_expected = fn(&'a isize)
let t_expected = {
let t_ptr_bound2 = env.t_rptr_late_bound_with_debruijn(1, ty::DebruijnIndex::new(2));
let t_ptr_bound2 = env.t_rptr_late_bound_with_debruijn(1, ty::DebruijnIndex::new(1));
env.t_fn(&[t_ptr_bound2], env.t_nil())
};

Expand Down Expand Up @@ -521,7 +521,7 @@ fn subst_ty_renumber_some_bounds() {
//
// but not that the Debruijn index is different in the different cases.
let t_expected = {
let t_rptr_bound2 = env.t_rptr_late_bound_with_debruijn(1, ty::DebruijnIndex::new(2));
let t_rptr_bound2 = env.t_rptr_late_bound_with_debruijn(1, ty::DebruijnIndex::new(1));
env.t_pair(t_rptr_bound1, env.t_fn(&[t_rptr_bound2], env.t_nil()))
};

Expand Down Expand Up @@ -549,10 +549,10 @@ fn escaping() {
let t_rptr_free1 = env.t_rptr_free(1);
assert!(!t_rptr_free1.has_escaping_regions());

let t_rptr_bound1 = env.t_rptr_late_bound_with_debruijn(1, ty::DebruijnIndex::new(1));
let t_rptr_bound1 = env.t_rptr_late_bound_with_debruijn(1, ty::DebruijnIndex::new(0));
assert!(t_rptr_bound1.has_escaping_regions());

let t_rptr_bound2 = env.t_rptr_late_bound_with_debruijn(1, ty::DebruijnIndex::new(2));
let t_rptr_bound2 = env.t_rptr_late_bound_with_debruijn(1, ty::DebruijnIndex::new(1));
assert!(t_rptr_bound2.has_escaping_regions());

// t_fn = fn(A)
Expand All @@ -568,7 +568,7 @@ fn escaping() {
#[test]
fn subst_region_renumber_region() {
test_env(EMPTY_SOURCE_STR, errors(&[]), |env| {
let re_bound1 = env.re_late_bound_with_debruijn(1, ty::DebruijnIndex::new(1));
let re_bound1 = env.re_late_bound_with_debruijn(1, ty::DebruijnIndex::new(0));

// type t_source<'a> = fn(&'a isize)
let t_source = {
Expand All @@ -583,7 +583,7 @@ fn subst_region_renumber_region() {
//
// but not that the Debruijn index is different in the different cases.
let t_expected = {
let t_rptr_bound2 = env.t_rptr_late_bound_with_debruijn(1, ty::DebruijnIndex::new(2));
let t_rptr_bound2 = env.t_rptr_late_bound_with_debruijn(1, ty::DebruijnIndex::new(1));
env.t_fn(&[t_rptr_bound2], env.t_nil())
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ pub fn ty_fn_sig<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
let tcx = cx.tcx;
let sig = substs.generator_poly_sig(def_id, cx.tcx);

let env_region = ty::ReLateBound(ty::DebruijnIndex::new(1), ty::BrEnv);
let env_region = ty::ReLateBound(ty::DebruijnIndex::new(0), ty::BrEnv);
let env_ty = tcx.mk_mut_ref(tcx.mk_region(env_region), ty);

sig.map_bound(|sig| {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/check/intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ pub fn check_intrinsic_type<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
"pref_align_of" | "min_align_of" => (1, Vec::new(), tcx.types.usize),
"size_of_val" | "min_align_of_val" => {
(1, vec![
tcx.mk_imm_ref(tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(1),
tcx.mk_imm_ref(tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(0),
ty::BrAnon(0))),
param(0))
], tcx.types.usize)
Expand Down Expand Up @@ -298,7 +298,7 @@ pub fn check_intrinsic_type<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
"unlikely" => (0, vec![tcx.types.bool], tcx.types.bool),

"discriminant_value" => (1, vec![
tcx.mk_imm_ref(tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(1),
tcx.mk_imm_ref(tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(0),
ty::BrAnon(0))),
param(0))], tcx.types.u64),

Expand Down