Skip to content

Commit

Permalink
use static as object-lifetime default for type XX in Foo<Item=XX>
Browse files Browse the repository at this point in the history
Currently the default is "inherited" from context, so e.g.  `&impl
Foo<Item = dyn Bar>` would default to `&'x impl Foo<Item = dyn Bar +
'x>`, but this triggers an ICE and is not very consistent.

This patch doesn't implement what I expect would be the correct
semantics, because those are likely too complex. Instead, it handles
what I'd expect to be the common case -- where the trait has no
lifetime parameters.
  • Loading branch information
nikomatsakis committed Aug 12, 2019
1 parent 282cc02 commit afa4a2a
Show file tree
Hide file tree
Showing 9 changed files with 229 additions and 1 deletion.
71 changes: 70 additions & 1 deletion src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {

fn visit_ty(&mut self, ty: &'tcx hir::Ty) {
debug!("visit_ty: id={:?} ty={:?}", ty.hir_id, ty);
debug!("visit_ty: ty.node={:?}", ty.node);
match ty.node {
hir::TyKind::BareFn(ref c) => {
let next_early_index = self.next_early_index();
Expand Down Expand Up @@ -1923,6 +1924,13 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}

fn visit_segment_args(&mut self, res: Res, depth: usize, generic_args: &'tcx hir::GenericArgs) {
debug!(
"visit_segment_args(res={:?}, depth={:?}, generic_args={:?})",
res,
depth,
generic_args,
);

if generic_args.parenthesized {
let was_in_fn_syntax = self.is_in_fn_syntax;
self.is_in_fn_syntax = true;
Expand Down Expand Up @@ -1976,6 +1984,23 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
_ => None,
};

debug!("visit_segment_args: type_def_id={:?}", type_def_id);

// Compute a vector of defaults, one for each type parameter,
// per the rules given in RFCs 599 and 1156. Example:
//
// ```rust
// struct Foo<'a, T: 'a, U> { }
// ```
//
// If you have `Foo<'x, dyn Bar, dyn Baz>`, we want to default
// `dyn Bar` to `dyn Bar + 'x` (because of the `T: 'a` bound)
// and `dyn Baz` to `dyn Baz + 'static` (because there is no
// such bound).
//
// Therefore, we would compute `object_lifetime_defaults` to a
// vector like `['x, 'static]`. Note that the vector only
// includes type parameters.
let object_lifetime_defaults = type_def_id.map_or(vec![], |def_id| {
let in_body = {
let mut scope = self.scope;
Expand Down Expand Up @@ -2015,6 +2040,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
.collect()
})
};
debug!("visit_segment_args: unsubst={:?}", unsubst);
unsubst
.iter()
.map(|set| match *set {
Expand All @@ -2035,6 +2061,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
.collect()
});

debug!("visit_segment_args: object_lifetime_defaults={:?}", object_lifetime_defaults);

let mut i = 0;
for arg in &generic_args.args {
match arg {
Expand All @@ -2057,8 +2085,49 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}
}

// Hack: when resolving the type `XX` in binding like `dyn
// Foo<'b, Item = XX>`, the current object-lifetime default
// would be to examine the trait `Foo` to check whether it has
// a lifetime bound declared on `Item`. e.g., if `Foo` is
// declared like so, then the default object lifetime bound in
// `XX` should be `'b`:
//
// ```rust
// trait Foo<'a> {
// type Item: 'a;
// }
// ```
//
// but if we just have `type Item;`, then it would be
// `'static`. However, we don't get all of this logic correct.
//
// Instead, we do something hacky: if there are no lifetime parameters
// to the trait, then we simply use a default object lifetime
// bound of `'static`, because there is no other possibility. On the other hand,
// if there ARE lifetime parameters, then we require the user to give an
// explicit bound for now.
//
// This is intended to leave room for us to implement the
// correct behavior in the future.
let has_lifetime_parameter = generic_args
.args
.iter()
.any(|arg| match arg {
GenericArg::Lifetime(_) => true,
_ => false,
});

// Resolve lifetimes found in the type `XX` from `Item = XX` bindings.
for b in &generic_args.bindings {
self.visit_assoc_type_binding(b);
let scope = Scope::ObjectLifetimeDefault {
lifetime: if has_lifetime_parameter {
None
} else {
Some(Region::Static)
},
s: self.scope,
};
self.with(scope, |_, this| this.visit_assoc_type_binding(b));
}
}

Expand Down
27 changes: 27 additions & 0 deletions src/test/ui/impl-trait/dyn-trait-elided-two-inputs-ref-assoc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Test that we don't get an error with `dyn Bar` in an impl Trait
// when there are multiple inputs. The `dyn Bar` should default to `+
// 'static`. This used to erroneously generate an error (cc #62517).
//
// check-pass

trait Foo {
type Item: ?Sized;

fn item(&self) -> Box<Self::Item> { panic!() }
}

trait Bar { }

impl<T> Foo for T {
type Item = dyn Bar;
}

fn is_static<T>(_: T) where T: 'static { }

fn bar(x: &str) -> &impl Foo<Item = dyn Bar> { &() }

fn main() {
let s = format!("foo");
let r = bar(&s);
is_static(r.item());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Test that `dyn Bar<Item = XX>` uses `'static` as the default object
// lifetime bound for the type `XX`.

trait Foo<'a> {
type Item: ?Sized;

fn item(&self) -> Box<Self::Item> { panic!() }
}

trait Bar { }

impl<T> Foo<'_> for T {
type Item = dyn Bar;
}

fn is_static<T>(_: T) where T: 'static { }

// Here, we should default to `dyn Bar + 'static`, but the current
// code forces us into a conservative, hacky path.
fn bar<'a>(x: &'a str) -> &'a dyn Foo<'a, Item = dyn Bar> { &() }
//~^ ERROR please supply an explicit bound

fn main() {
let s = format!("foo");
let r = bar(&s);
is_static(r.item());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error[E0228]: the lifetime bound for this object type cannot be deduced from context; please supply an explicit bound
--> $DIR/object-lifetime-default-dyn-binding-nonstatic1.rs:20:50
|
LL | fn bar<'a>(x: &'a str) -> &'a dyn Foo<'a, Item = dyn Bar> { &() }
| ^^^^^^^

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Test that `dyn Bar<Item = XX>` uses `'static` as the default object
// lifetime bound for the type `XX`.

trait Foo<'a> {
type Item: 'a + ?Sized;

fn item(&self) -> Box<Self::Item> { panic!() }
}

trait Bar { }

impl<T> Foo<'_> for T {
type Item = dyn Bar;
}

fn is_static<T>(_: T) where T: 'static { }

// Here, we default to `dyn Bar + 'a`. Or, we *should*, but the
// current code forces us into a conservative, hacky path.
fn bar<'a>(x: &'a str) -> &'a dyn Foo<'a, Item = dyn Bar> { &() }
//~^ ERROR please supply an explicit bound

fn main() {
let s = format!("foo");
let r = bar(&s);

// If it weren't for the conservative path above, we'd expect an
// error here.
is_static(r.item());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error[E0228]: the lifetime bound for this object type cannot be deduced from context; please supply an explicit bound
--> $DIR/object-lifetime-default-dyn-binding-nonstatic2.rs:20:50
|
LL | fn bar<'a>(x: &'a str) -> &'a dyn Foo<'a, Item = dyn Bar> { &() }
| ^^^^^^^

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Test that `dyn Bar<Item = XX>` uses `'static` as the default object
// lifetime bound for the type `XX`.

trait Foo<'a> {
type Item: ?Sized;

fn item(&self) -> Box<Self::Item> { panic!() }
}

trait Bar { }

fn is_static<T>(_: T) where T: 'static { }

// Here, we should default to `dyn Bar + 'static`, but the current
// code forces us into a conservative, hacky path.
fn bar(x: &str) -> &dyn Foo<Item = dyn Bar> { &() }
//~^ ERROR please supply an explicit bound

fn main() {
let s = format!("foo");
let r = bar(&s);
is_static(r.item());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error[E0228]: the lifetime bound for this object type cannot be deduced from context; please supply an explicit bound
--> $DIR/object-lifetime-default-dyn-binding-nonstatic3.rs:16:36
|
LL | fn bar(x: &str) -> &dyn Foo<Item = dyn Bar> { &() }
| ^^^^^^^

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Test that `dyn Bar<Item = XX>` uses `'static` as the default object
// lifetime bound for the type `XX`.
//
// check-pass

trait Foo {
type Item: ?Sized;

fn item(&self) -> Box<Self::Item> { panic!() }
}

trait Bar { }

impl<T> Foo for T {
type Item = dyn Bar;
}

fn is_static<T>(_: T) where T: 'static { }

// Here, we default to `dyn Bar + 'static`, and not `&'x dyn Foo<Item
// = dyn Bar + 'x>`.
fn bar(x: &str) -> &dyn Foo<Item = dyn Bar> { &() }

fn main() {
let s = format!("foo");
let r = bar(&s);
is_static(r.item());
}

0 comments on commit afa4a2a

Please sign in to comment.