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

Lifetime Resolution for Generic Associated Types #46706

Merged
merged 4 commits into from
Dec 15, 2017
Merged

Lifetime Resolution for Generic Associated Types #46706

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8cfaf1b
Trait item lifetime resolution for GATs
sunjay Dec 13, 2017
4f90eac
Updating tests to remove all "undeclared lifetime" errors (since thos…
sunjay Dec 13, 2017
b90e9c9
Added test to make sure we can refer to the declared traits of a gene…
sunjay Dec 13, 2017
f701b4c
Added test to make sure that undeclared lifetimes are in fact detected
sunjay Dec 13, 2017
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
87 changes: 67 additions & 20 deletions src/librustc/middle/resolve_lifetime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,30 +624,77 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
}

fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem) {
let tcx = self.tcx;
if let hir::TraitItemKind::Method(ref sig, _) = trait_item.node {
self.visit_early_late(
Some(tcx.hir.get_parent(trait_item.id)),
&sig.decl,
&trait_item.generics,
|this| intravisit::walk_trait_item(this, trait_item),
)
} else {
intravisit::walk_trait_item(self, trait_item);
use self::hir::TraitItemKind::*;
match trait_item.node {
Method(ref sig, _) => {
let tcx = self.tcx;
self.visit_early_late(
Some(tcx.hir.get_parent(trait_item.id)),
&sig.decl,
&trait_item.generics,
|this| intravisit::walk_trait_item(this, trait_item),
);
},
Type(ref bounds, ref ty) => {
let generics = &trait_item.generics;
let mut index = self.next_early_index();
debug!("visit_ty: index = {}", index);
let lifetimes = generics.lifetimes.iter()
.map(|lt_def| Region::early(&self.tcx.hir, &mut index, lt_def))
.collect();

let next_early_index = index + generics.ty_params.len() as u32;
let scope = Scope::Binder { lifetimes, next_early_index, s: self.scope };
self.with(scope, |_old_scope, this| {
this.visit_generics(generics);
for bound in bounds {
this.visit_ty_param_bound(bound);
}
if let Some(ty) = ty {
this.visit_ty(ty);
}
});
},
Const(_, _) => {
// Only methods and types support generics.
assert!(!trait_item.generics.is_parameterized());
Copy link
Contributor

Choose a reason for hiding this comment

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

good to see the assert =)

intravisit::walk_trait_item(self, trait_item);
},
}
}

fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem) {
let tcx = self.tcx;
if let hir::ImplItemKind::Method(ref sig, _) = impl_item.node {
self.visit_early_late(
Some(tcx.hir.get_parent(impl_item.id)),
&sig.decl,
&impl_item.generics,
|this| intravisit::walk_impl_item(this, impl_item),
)
} else {
intravisit::walk_impl_item(self, impl_item);
use self::hir::ImplItemKind::*;
match impl_item.node {
Method(ref sig, _) => {
let tcx = self.tcx;
self.visit_early_late(
Some(tcx.hir.get_parent(impl_item.id)),
&sig.decl,
&impl_item.generics,
|this| intravisit::walk_impl_item(this, impl_item),
)
},
Type(ref ty) => {
let generics = &impl_item.generics;
let mut index = self.next_early_index();
debug!("visit_ty: index = {}", index);
let lifetimes = generics.lifetimes.iter()
.map(|lt_def| Region::early(&self.tcx.hir, &mut index, lt_def))
.collect();

let next_early_index = index + generics.ty_params.len() as u32;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessarily something we should fix in this PR, but it does feel like we have a lot of code duplication here, don't it? Feels like we want a "introduce scope w/ early-bound generics" helper function or something, but I'd have to look around to see how many cases could readily use such a thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I was thinking about that too. I'll need to look into how many cases actually use a similar pattern. If it's just these two, it may not be worth it. Even these two cases do subtly different things (e.g. no bounds checking in impl items)

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked and didn't see a lot of places that are using this pattern. Let me know if you would like me to do anything about the small bit of repetition here. =)

let scope = Scope::Binder { lifetimes, next_early_index, s: self.scope };
self.with(scope, |_old_scope, this| {
this.visit_generics(generics);
this.visit_ty(ty);
});
},
Const(_, _) => {
// Only methods and types support generics.
assert!(!impl_item.generics.is_parameterized());
intravisit::walk_impl_item(self, impl_item);
},
}
}

Expand Down