Skip to content

Commit

Permalink
Auto merge of #46616 - cramertj:impl-trait-elision, r=nikomatsakis
Browse files Browse the repository at this point in the history
Implement impl Trait lifetime elision

Fixes #43396.

There's one weird ICE in the interaction with argument-position `impl Trait`. I'm still debugging it-- I've left a test for it commented out with a FIXME.

Also included a FIXME to ensure that `impl Trait` traits are caught under the lint in #45992.

r? @nikomatsakis
  • Loading branch information
bors committed Dec 13, 2017
2 parents 691f022 + 018c403 commit dcf3db4
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 56 deletions.
112 changes: 73 additions & 39 deletions src/librustc/hir/lowering.rs
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
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 };
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
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

0 comments on commit dcf3db4

Please sign in to comment.