From 832199ee767004091d083affb5e641502f6d39bc Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 12 Aug 2019 10:41:05 -0400 Subject: [PATCH] use static as object-lifetime default for type XX in `Foo` Currently the default is "inherited" from context, so e.g. `&impl Foo` would default to `&'x impl Foo`, 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. --- src/librustc/middle/resolve_lifetime.rs | 71 ++++++++++++++++++- .../dyn-trait-elided-two-inputs-ref-assoc.rs | 27 +++++++ ...lifetime-default-dyn-binding-nonstatic1.rs | 27 +++++++ ...time-default-dyn-binding-nonstatic1.stderr | 8 +++ ...lifetime-default-dyn-binding-nonstatic2.rs | 30 ++++++++ ...time-default-dyn-binding-nonstatic2.stderr | 8 +++ ...lifetime-default-dyn-binding-nonstatic3.rs | 23 ++++++ ...time-default-dyn-binding-nonstatic3.stderr | 8 +++ ...ect-lifetime-default-dyn-binding-static.rs | 28 ++++++++ 9 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/impl-trait/dyn-trait-elided-two-inputs-ref-assoc.rs create mode 100644 src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic1.rs create mode 100644 src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic1.stderr create mode 100644 src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic2.rs create mode 100644 src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic2.stderr create mode 100644 src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic3.rs create mode 100644 src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic3.stderr create mode 100644 src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-static.rs diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index 3b604bef78e..f5b0af61693 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -558,6 +558,7 @@ fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem) { 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(); @@ -1923,6 +1924,13 @@ fn resolve_lifetime_ref(&mut self, lifetime_ref: &'tcx hir::Lifetime) { } 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; @@ -1976,6 +1984,23 @@ fn visit_segment_args(&mut self, res: Res, depth: usize, generic_args: &'tcx hir _ => 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; @@ -2015,6 +2040,7 @@ fn visit_segment_args(&mut self, res: Res, depth: usize, generic_args: &'tcx hir .collect() }) }; + debug!("visit_segment_args: unsubst={:?}", unsubst); unsubst .iter() .map(|set| match *set { @@ -2035,6 +2061,8 @@ fn visit_segment_args(&mut self, res: Res, depth: usize, generic_args: &'tcx hir .collect() }); + debug!("visit_segment_args: object_lifetime_defaults={:?}", object_lifetime_defaults); + let mut i = 0; for arg in &generic_args.args { match arg { @@ -2057,8 +2085,49 @@ fn visit_segment_args(&mut self, res: Res, depth: usize, generic_args: &'tcx hir } } + // 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)); } } diff --git a/src/test/ui/impl-trait/dyn-trait-elided-two-inputs-ref-assoc.rs b/src/test/ui/impl-trait/dyn-trait-elided-two-inputs-ref-assoc.rs new file mode 100644 index 00000000000..aad9d89fe24 --- /dev/null +++ b/src/test/ui/impl-trait/dyn-trait-elided-two-inputs-ref-assoc.rs @@ -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 { panic!() } +} + +trait Bar { } + +impl Foo for T { + type Item = dyn Bar; +} + +fn is_static(_: T) where T: 'static { } + +fn bar(x: &str) -> &impl Foo { &() } + +fn main() { + let s = format!("foo"); + let r = bar(&s); + is_static(r.item()); +} diff --git a/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic1.rs b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic1.rs new file mode 100644 index 00000000000..7337383e297 --- /dev/null +++ b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic1.rs @@ -0,0 +1,27 @@ +// Test that `dyn Bar` uses `'static` as the default object +// lifetime bound for the type `XX`. + +trait Foo<'a> { + type Item: ?Sized; + + fn item(&self) -> Box { panic!() } +} + +trait Bar { } + +impl Foo<'_> for T { + type Item = dyn Bar; +} + +fn is_static(_: 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()); +} diff --git a/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic1.stderr b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic1.stderr new file mode 100644 index 00000000000..9dbf7a78ed7 --- /dev/null +++ b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic1.stderr @@ -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 + diff --git a/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic2.rs b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic2.rs new file mode 100644 index 00000000000..2a7415174f8 --- /dev/null +++ b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic2.rs @@ -0,0 +1,30 @@ +// Test that `dyn Bar` uses `'static` as the default object +// lifetime bound for the type `XX`. + +trait Foo<'a> { + type Item: 'a + ?Sized; + + fn item(&self) -> Box { panic!() } +} + +trait Bar { } + +impl Foo<'_> for T { + type Item = dyn Bar; +} + +fn is_static(_: 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()); +} diff --git a/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic2.stderr b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic2.stderr new file mode 100644 index 00000000000..d069f52ce47 --- /dev/null +++ b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic2.stderr @@ -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 + diff --git a/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic3.rs b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic3.rs new file mode 100644 index 00000000000..51be999a632 --- /dev/null +++ b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic3.rs @@ -0,0 +1,23 @@ +// Test that `dyn Bar` uses `'static` as the default object +// lifetime bound for the type `XX`. + +trait Foo<'a> { + type Item: ?Sized; + + fn item(&self) -> Box { panic!() } +} + +trait Bar { } + +fn is_static(_: 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 { &() } +//~^ ERROR please supply an explicit bound + +fn main() { + let s = format!("foo"); + let r = bar(&s); + is_static(r.item()); +} diff --git a/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic3.stderr b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic3.stderr new file mode 100644 index 00000000000..9c7b6b98f2e --- /dev/null +++ b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic3.stderr @@ -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 { &() } + | ^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-static.rs b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-static.rs new file mode 100644 index 00000000000..339f3356bd7 --- /dev/null +++ b/src/test/ui/object-lifetime/object-lifetime-default-dyn-binding-static.rs @@ -0,0 +1,28 @@ +// Test that `dyn Bar` uses `'static` as the default object +// lifetime bound for the type `XX`. +// +// check-pass + +trait Foo { + type Item: ?Sized; + + fn item(&self) -> Box { panic!() } +} + +trait Bar { } + +impl Foo for T { + type Item = dyn Bar; +} + +fn is_static(_: T) where T: 'static { } + +// Here, we default to `dyn Bar + 'static`, and not `&'x dyn Foo`. +fn bar(x: &str) -> &dyn Foo { &() } + +fn main() { + let s = format!("foo"); + let r = bar(&s); + is_static(r.item()); +} -- 2.44.0