Skip to content

Implement impl Trait lifetime elision #46616

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

Merged
merged 1 commit into from
Dec 13, 2017
Merged

Implement impl Trait lifetime elision #46616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
112 changes: 73 additions & 39 deletions src/librustc/hir/lowering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -990,8 +990,9 @@ impl<'a> LoweringContext<'a> {
struct ImplTraitLifetimeCollector<'r, 'a: 'r> {
context: &'r mut LoweringContext<'a>,
parent: DefIndex,
currently_bound_lifetimes: Vec<Name>,
already_defined_lifetimes: HashSet<Name>,
collect_elided_lifetimes: bool,
currently_bound_lifetimes: Vec<hir::LifetimeName>,
already_defined_lifetimes: HashSet<hir::LifetimeName>,
output_lifetimes: Vec<hir::Lifetime>,
output_lifetime_defs: Vec<hir::LifetimeDef>,
}
Expand All @@ -1002,6 +1003,30 @@ impl<'a> LoweringContext<'a> {
hir::intravisit::NestedVisitorMap::None
}

fn visit_path_parameters(&mut self, span: Span, parameters: &'v hir::PathParameters) {
// Don't collect elided lifetimes used inside of `Fn()` syntax.
if parameters.parenthesized {
let old_collect_elided_lifetimes = self.collect_elided_lifetimes;
self.collect_elided_lifetimes = false;
hir::intravisit::walk_path_parameters(self, span, parameters);
self.collect_elided_lifetimes = old_collect_elided_lifetimes;
} else {
hir::intravisit::walk_path_parameters(self, span, parameters);
}
}

fn visit_ty(&mut self, t: &'v hir::Ty) {
// Don't collect elided lifetimes used inside of `fn()` syntax
if let &hir::Ty_::TyBareFn(_) = &t.node {
let old_collect_elided_lifetimes = self.collect_elided_lifetimes;
self.collect_elided_lifetimes = false;
hir::intravisit::walk_ty(self, t);
self.collect_elided_lifetimes = old_collect_elided_lifetimes;
} else {
hir::intravisit::walk_ty(self, t);
}
}

fn visit_poly_trait_ref(&mut self,
polytr: &'v hir::PolyTraitRef,
_: hir::TraitBoundModifier) {
Expand All @@ -1010,10 +1035,8 @@ impl<'a> LoweringContext<'a> {
// Record the introduction of 'a in `for<'a> ...`
for lt_def in &polytr.bound_lifetimes {
// Introduce lifetimes one at a time so that we can handle
// cases like `fn foo<'d>() -> impl for<'a, 'b: 'a, 'c: 'b + 'd> ...`
if let hir::LifetimeName::Name(name) = lt_def.lifetime.name {
self.currently_bound_lifetimes.push(name);
}
// cases like `fn foo<'d>() -> impl for<'a, 'b: 'a, 'c: 'b + 'd>`
self.currently_bound_lifetimes.push(lt_def.lifetime.name);

// Visit the lifetime bounds
for lt_bound in &lt_def.bounds {
Expand All @@ -1027,47 +1050,58 @@ impl<'a> LoweringContext<'a> {
}

fn visit_lifetime(&mut self, lifetime: &'v hir::Lifetime) {
// Exclude '_, 'static, and elided lifetimes (there should be no elided lifetimes)
if let hir::LifetimeName::Name(lifetime_name) = lifetime.name {
if !self.currently_bound_lifetimes.contains(&lifetime_name) &&
!self.already_defined_lifetimes.contains(&lifetime_name)
{
self.already_defined_lifetimes.insert(lifetime_name);
let name = hir::LifetimeName::Name(lifetime_name);

self.output_lifetimes.push(hir::Lifetime {
id: self.context.next_id().node_id,
span: lifetime.span,
name,
});
let name = match lifetime.name {
hir::LifetimeName::Implicit |
hir::LifetimeName::Underscore =>
if self.collect_elided_lifetimes {
// Use `'_` for both implicit and underscore lifetimes in
// `abstract type Foo<'_>: SomeTrait<'_>;`
hir::LifetimeName::Underscore
} else {
return
}
name @ hir::LifetimeName::Name(_) => name,
hir::LifetimeName::Static => return,
};

let def_node_id = self.context.next_id().node_id;
self.context.resolver.definitions().create_def_with_parent(
self.parent,
def_node_id,
DefPathData::LifetimeDef(lifetime_name.as_str()),
DefIndexAddressSpace::High,
Mark::root()
);
let def_lifetime = hir::Lifetime {
id: def_node_id,
span: lifetime.span,
name,
};
self.output_lifetime_defs.push(hir::LifetimeDef {
lifetime: def_lifetime,
bounds: Vec::new().into(),
pure_wrt_drop: false,
in_band: false,
});
}
if !self.currently_bound_lifetimes.contains(&name) &&
!self.already_defined_lifetimes.contains(&name)
{
self.already_defined_lifetimes.insert(name);

self.output_lifetimes.push(hir::Lifetime {
id: self.context.next_id().node_id,
span: lifetime.span,
name,
});

let def_node_id = self.context.next_id().node_id;
self.context.resolver.definitions().create_def_with_parent(
self.parent,
def_node_id,
DefPathData::LifetimeDef(name.name().as_str()),
DefIndexAddressSpace::High,
Mark::root()
);
let def_lifetime = hir::Lifetime {
id: def_node_id,
span: lifetime.span,
name: name,
};
self.output_lifetime_defs.push(hir::LifetimeDef {
lifetime: def_lifetime,
bounds: Vec::new().into(),
pure_wrt_drop: false,
in_band: false,
});
}
}
}

let mut lifetime_collector = ImplTraitLifetimeCollector {
context: self,
parent: parent_index,
collect_elided_lifetimes: true,
currently_bound_lifetimes: Vec::new(),
already_defined_lifetimes: HashSet::new(),
output_lifetimes: Vec::new(),
Expand Down
53 changes: 37 additions & 16 deletions src/librustc/middle/resolve_lifetime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,24 +600,45 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
} = *exist_ty;
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);
let mut elision = None;
let mut lifetimes = FxHashMap();
for lt_def in &generics.lifetimes {
let (lt_name, region) = Region::early(&self.tcx.hir, &mut index, &lt_def);
if let hir::LifetimeName::Underscore = lt_name {
// Pick the elided lifetime "definition" if one exists and use it to make an
// elision scope.
elision = Some(region);
} else {
lifetimes.insert(lt_name, region);
}
});
}

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

if let Some(elision_region) = elision {
let scope = Scope::Elision {
elide: Elide::Exact(elision_region),
s: self.scope
};
self.with(scope, |_old_scope, this| {
let scope = Scope::Binder { lifetimes, next_early_index, s: this.scope };
Copy link
Member Author

@cramertj cramertj Dec 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

There's a bit of duplicated code here, but I couldn't figure out a good way to factor it out-- it seems too little code for its own function, but a closure would have a signature like for<'a, 'tcx> Fn(&mut LifetimeContext<'a, 'tcx>, &'tcx Generics) and I don't believe there's a way to write that ATM (though I could be wrong)

this.with(scope, |_old_scope, this| {
this.visit_generics(generics);
for bound in bounds {
this.visit_ty_param_bound(bound);
}
});
});
} else {
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);
}
});
}
}
_ => intravisit::walk_ty(self, ty),
}
Expand Down
26 changes: 25 additions & 1 deletion src/test/run-pass/impl-trait/lifetimes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(conservative_impl_trait)]
#![feature(conservative_impl_trait, underscore_lifetimes, universal_impl_trait)]
#![allow(warnings)]

use std::fmt::Debug;
Expand All @@ -32,12 +32,36 @@ fn no_params_or_lifetimes_is_static() -> impl Debug + 'static {
fn static_input_type_is_static<T: Debug + 'static>(x: T) -> impl Debug + 'static { x }

fn type_outlives_reference_lifetime<'a, T: Debug>(x: &'a T) -> impl Debug + 'a { x }
fn type_outlives_reference_lifetime_elided<T: Debug>(x: &T) -> impl Debug + '_ { x }

trait SingleRegionTrait<'a> {}
impl<'a> SingleRegionTrait<'a> for u32 {}
impl<'a> SingleRegionTrait<'a> for &'a u32 {}
struct SingleRegionStruct<'a>(&'a u32);

fn simple_type_hrtb<'b>() -> impl for<'a> SingleRegionTrait<'a> { 5 }
// FIXME(cramertj) add test after #45992 lands to ensure lint is triggered
fn elision_single_region_trait(x: &u32) -> impl SingleRegionTrait { x }
fn elision_single_region_struct(x: SingleRegionStruct) -> impl Into<SingleRegionStruct> { x }

fn closure_hrtb() -> impl for<'a> Fn(&'a u32) { |_| () }
fn closure_hr_elided() -> impl Fn(&u32) { |_| () }
fn closure_hr_elided_return() -> impl Fn(&u32) -> &u32 { |x| x }
fn closure_pass_through_elided_return(x: impl Fn(&u32) -> &u32) -> impl Fn(&u32) -> &u32 { x }
fn closure_pass_through_reference_elided(x: &impl Fn(&u32) -> &u32) -> &impl Fn(&u32) -> &u32 { x }

fn pass_through_elision(x: &u32) -> impl Into<&u32> { x }
fn pass_through_elision_with_fn_ptr(x: &fn(&u32) -> &u32) -> impl Into<&fn(&u32) -> &u32> { x }

fn pass_through_elision_with_fn_path<T: Fn(&u32) -> &u32>(
x: &T
) -> impl Into<&impl Fn(&u32) -> &u32> { x }

// FIXME(cramertj) Currently ICEing, part of issue #46685:
// fn foo(x: &impl Debug) -> impl Into<&impl Debug> { x }
// Works:
fn foo_no_outer_impl(x: &impl Debug) -> &impl Debug { x }
fn foo_explicit_arg<T: Debug>(x: &T) -> impl Into<&impl Debug> { x }

fn mixed_lifetimes<'a>() -> impl for<'b: 'a> Fn(&'b u32) { |_| () }
fn mixed_as_static() -> impl Fn(&'static u32) { mixed_lifetimes() }
Expand Down