-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Make trait refs & assoc ty paths properly induce trait object lifetime defaults #129543
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.
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
Open
fmease
wants to merge
8
commits into
rust-lang:main
Choose a base branch
from
fmease:obj-lt-def-gat
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
1ed1b73
Make resolved GAT paths induce trait object lifetime defaults for the…
fmease e891f94
Split out fn `compute_object_lifetime_defaults`
fmease c939fab
Don't needlessly search for already-found HIR generic param
fmease 74b15be
Make resolved assoc ty paths induce a trait object lifetime default f…
fmease 665e1e2
Tweak `#[rustc_dump_object_lifetime_defaults]`
fmease 4eddfb0
Rename `hir::GenericArgs::{num,has}_generic_params` to `*_generic_args`
fmease a07e8e6
Render trait object lifetime defaults indeterminate in lifetime-insta…
fmease 9b4931c
Introduce fn `eligible_container` & rename depth to rev seg index
fmease File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
543 changes: 353 additions & 190 deletions
543
compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
33 changes: 33 additions & 0 deletions
33
tests/ui/object-lifetime/object-lifetime-default-assoc-ty-binding-item-bounds-non-static.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| // Ideally, given an assoc type binding `dyn Trait<AssocTy = Ty>`, we'd factor in the item bounds of | ||
| // assoc type `AssocTy` when computing the trait object lifetime default for type `Ty`. | ||
| // | ||
| // However, since the current implementation can't handle this we instead conservatively and hackily | ||
| // treat the trait object lifetime default of the RHS as indeterminate if any lifetime arguments are | ||
| // passed to the trait ref (or the GAT) thus rejecting any implicit trait object lifetime bounds. | ||
| // This way, we can still implement the desired behavior in the future. | ||
|
|
||
| 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 {} | ||
|
|
||
| // FIXME: Ideally, we'd elaborate `dyn Bar` to `dyn Bar + 'a` instead of rejecting it. | ||
| fn bar<'a>(x: &'a str) -> &'a dyn Foo<'a, Item = dyn Bar> { &() } | ||
| //~^ ERROR cannot deduce the lifetime bound for this trait object type from context | ||
|
|
||
| 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()); | ||
| } |
2 changes: 1 addition & 1 deletion
2
...ime-default-dyn-binding-nonstatic1.stderr → ...-ty-binding-item-bounds-non-static.stderr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
30 changes: 30 additions & 0 deletions
30
tests/ui/object-lifetime/object-lifetime-default-assoc-ty-binding-item-bounds-static.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| // Check that assoc item bindings correctly induce trait object lifetime defaults `'static` if the | ||
| // the trait & assoc ty doesn't have any lifetime params & the assoc ty isn't bounded by a lifetime. | ||
| // | ||
| //@ check-pass | ||
|
|
||
| trait Foo { | ||
| type Item: ?Sized; | ||
|
|
||
| fn item(&self) -> Box<Self::Item> { loop {} } | ||
| } | ||
|
|
||
| trait Bar {} | ||
|
|
||
| impl<T> Foo for T { | ||
| type Item = dyn Bar; | ||
| } | ||
|
|
||
| fn is_static<T>(_: T) where T: 'static {} | ||
|
|
||
| // We elaborate `dyn Bar` to `dyn Bar + 'static` since the assoc ty isn't bounded by any lifetime. | ||
| // Notably, we don't elaborate it to `dyn Bar + 'r` since the trait object lifetime default induced | ||
| // by `Foo` (i.e., `'static`) shadows the one induced by `&` (`'r`). | ||
| fn bar<'r>(x: &'r str) -> &'r dyn Foo<Item = dyn Bar> { &() } | ||
|
|
||
| fn main() { | ||
| let s = format!("foo"); | ||
| let r = bar(&s); | ||
|
|
||
| is_static(r.item()); | ||
| } |
37 changes: 37 additions & 0 deletions
37
tests/ui/object-lifetime/object-lifetime-default-assoc-ty-binding-item-bounds.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // Ideally, given an assoc type binding `dyn Trait<AssocTy = Ty>`, we'd factor in the item bounds of | ||
| // assoc type `AssocTy` when computing the trait object lifetime default for type `Ty`. | ||
| // | ||
| // However, since the current implementation can't handle this we instead conservatively and hackily | ||
| // treat the trait object lifetime default of the RHS as indeterminate if any lifetime arguments are | ||
| // passed to the trait ref (or the GAT) thus rejecting any implicit trait object lifetime bounds. | ||
| // This way, we can still implement the desired behavior in the future. | ||
|
|
||
| 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 {} | ||
|
|
||
| // FIXME: Ideally, we'd elaborate `dyn Bar` to `dyn Bar + 'static` instead of rejecting it. | ||
| fn bar<'a>(x: &'a str) -> &'a dyn Foo<'a, Item = dyn Bar> { &() } | ||
| //~^ ERROR cannot deduce the lifetime bound for this trait object type from context | ||
|
|
||
| // FIXME: Ideally, we'd elaborate `dyn Bar` to `dyn Bar + 'static` instead of rejecting it. | ||
| fn baz(x: &str) -> &dyn Foo<Item = dyn Bar> { &() } | ||
| //~^ ERROR cannot deduce the lifetime bound for this trait object type from context | ||
|
|
||
| fn main() { | ||
| let s = format!("foo"); | ||
| let r = bar(&s); | ||
| is_static(r.item()); | ||
| let r = baz(&s); | ||
| is_static(r.item()); | ||
| } | ||
25 changes: 25 additions & 0 deletions
25
tests/ui/object-lifetime/object-lifetime-default-assoc-ty-binding-item-bounds.stderr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| error[E0228]: cannot deduce the lifetime bound for this trait object type from context | ||
| --> $DIR/object-lifetime-default-assoc-ty-binding-item-bounds.rs:24:50 | ||
| | | ||
| LL | fn bar<'a>(x: &'a str) -> &'a dyn Foo<'a, Item = dyn Bar> { &() } | ||
| | ^^^^^^^ | ||
| | | ||
| help: please supply an explicit bound | ||
| | | ||
| LL | fn bar<'a>(x: &'a str) -> &'a dyn Foo<'a, Item = dyn Bar + /* 'a */> { &() } | ||
| | ++++++++++ | ||
|
|
||
| error[E0228]: cannot deduce the lifetime bound for this trait object type from context | ||
| --> $DIR/object-lifetime-default-assoc-ty-binding-item-bounds.rs:28:36 | ||
| | | ||
| LL | fn baz(x: &str) -> &dyn Foo<Item = dyn Bar> { &() } | ||
| | ^^^^^^^ | ||
| | | ||
| help: please supply an explicit bound | ||
| | | ||
| LL | fn baz(x: &str) -> &dyn Foo<Item = dyn Bar + /* 'a */> { &() } | ||
| | ++++++++++ | ||
|
|
||
| error: aborting due to 2 previous errors | ||
|
|
||
| For more information about this error, try `rustc --explain E0228`. |
19 changes: 19 additions & 0 deletions
19
tests/ui/object-lifetime/object-lifetime-default-assoc-ty-self-ty-static.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // Check that resolved associated type paths induce the correct | ||
| // trait object lifetime default for the self type. | ||
|
|
||
| //@ check-pass | ||
|
|
||
| trait Outer { type Ty; } | ||
| trait Inner {} | ||
|
|
||
| impl<'a> Outer for dyn Inner + 'a { type Ty = &'a (); } | ||
|
|
||
| // We deduce `dyn Inner + 'static` from absence of any bounds on self ty param of trait `Outer`. | ||
| // | ||
| // Prior to PR rust-lang/rust#129543, assoc tys weren't considered eligible *containers* and | ||
| // thus we'd use the *trait object lifetime default* induced by the reference type ctor `&`, | ||
| // namely `'r`. Now however, the assoc ty overrides that default to be `'static`. | ||
| fn f<'r>(x: &'r <dyn Inner as Outer>::Ty) { /*check*/ g(x) } | ||
| fn g<'r>(x: &'r <dyn Inner + 'static as Outer>::Ty) {} | ||
|
|
||
| fn main() {} |
19 changes: 19 additions & 0 deletions
19
tests/ui/object-lifetime/object-lifetime-default-assoc-ty-self-ty.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // Check that resolved associated type paths induce the correct | ||
| // trait object lifetime default for the self type. | ||
|
|
||
| //@ check-pass | ||
| //@ revisions: bound clause | ||
|
|
||
| // RBV works on the HIR where where-clauses and item bounds of traits aren't merged yet. | ||
| // It's therefore wise to check both forms and make sure both are treated the same by RBV. | ||
| #[cfg(bound)] trait Outer<'a>: 'a { type Ty; } | ||
| #[cfg(clause)] trait Outer<'a> where Self: 'a { type Ty; } | ||
| trait Inner {} | ||
|
|
||
| impl<'a> Outer<'a> for dyn Inner + 'a { type Ty = &'a (); } | ||
|
|
||
| fn f<'r>(x: <dyn Inner + 'r as Outer<'r>>::Ty) { /*check*/ g(x) } | ||
| // We deduce `dyn Inner + 'r` from bound `'a` on self ty param of trait `Outer`. | ||
| fn g<'r>(x: <dyn Inner as Outer<'r>>::Ty) {} | ||
|
|
||
| fn main() {} |
27 changes: 0 additions & 27 deletions
27
tests/ui/object-lifetime/object-lifetime-default-dyn-binding-nonstatic1.rs
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we? 😅 I am not even sure whether this is desirable or not.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify? Which semantics would you desire?
AFAICT, there are 5 options for how we could make associated type bindings behave: The associated type binding could...
'staticunconditionally'staticsince OLD(Foo::Item) would beEmptywhich turns into'staticin item signaturesdyn Bartodyn Bar + 'asince it'd take on the default induced by the outer&<…>list?"Solo<·>!=(·,)wrt. obj lt defaulting); still, using that as a counterargument would probably be a stretch)'staticThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeterminate since OLD('staticFoo::Item) would beEmptywhich turns intoindeterminate for associated type bindings in item signatures'staticis what I'd expect
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Well, as a matter of fact in all other contexts Empty just like Static leads to a
'staticlifetime default in item signatures, so that would be a bit inconsistent. I mean maybe it's fine if type args and assoc type bindings get treated differently, idk.For comparison,
&'r Rc<dyn Trait>gets elab'ed to&'r Rc<dyn Trait + 'static>in item signatures sinceRc'sTis Empty. So it's neither indeterminate nor pass-through.