Move object lifetime default computation #97839
Conversation
rustbot
added
the
T-compiler
rust-highfive
added
the
S-waiting-on-review
There was a problem hiding this comment.
I'm not sure this is the right index here.
There was a problem hiding this comment.
This comment should be removed, I think.
compiler/rustc_typeck/src/collect.rs
Outdated
There was a problem hiding this comment.
Hmm. Is this true? I could imagine a scenario like
struct Foo<'a, T: 'a> {}
trait Bar {}
let _: for<'x> fn(Foo<'x, dyn Bar>);Does that apply here?
There was a problem hiding this comment.
This code is only used for function item-likes.
I agree, this is slightly wrong. However, this cannot cause a bug: ImplicitObjectLifetimeDefault can only reference a lifetime from an outer & reference, a generic argument, or 'static. So the lifetime this references has already been seen by the visitor.
However, this whole visitor will be useless once #97720 lands. All the lifetimes in a function's signature will be generic parameters, so only the loop in has_late_bound_regions will be required.
There was a problem hiding this comment.
Is this description correct?
It's probably worth elaborating a bit in a comment what the different between object_lifetime_map and object_lifetime_defaults are.
There was a problem hiding this comment.
It's probably worth elaborating a bit in a comment what the different between
object_lifetime_mapandobject_lifetime_defaultsare.
This is still relevant
src/test/ui/issues/issue-41139.rs
Outdated
There was a problem hiding this comment.
Why did this change? Not a huge deal, but would be nice to know why
There was a problem hiding this comment.
Wait, do you mean the existing test is a bug? Or the new output is?
There was a problem hiding this comment.
The new output is buggy.
cjgillot
force-pushed
the
object-lifetime-default-small
branch
from
fa5dff6 to
fc2713e
Compare
This comment has been minimized.
This comment has been minimized.
cjgillot
force-pushed
the
object-lifetime-default-small
branch
from
fc2713e to
5d5eda8
Compare
|
Gonna try to take another look at this PR this weekend. |
jackh726
left a comment
jackh726
left a comment
There was a problem hiding this comment.
Looking pretty good. Just two small comments. Also, can you merge some commits (pacify tidy, fix comments and query description) into other relevant commits. Not required, but would be nice. r=me after review addressed
There was a problem hiding this comment.
It's probably worth elaborating a bit in a comment what the different between
object_lifetime_mapandobject_lifetime_defaultsare.
This is still relevant
cjgillot
force-pushed
the
object-lifetime-default-small
branch
from
5d5eda8 to
1f535a0
Compare
This comment has been minimized.
This comment has been minimized.
cjgillot
force-pushed
the
object-lifetime-default-small
branch
from
1f535a0 to
37f42cb
Compare
|
I'm having doubts whether this is the right direction. I'm marking this as blocked until I have a proper idea of where we are going. |
cjgillot
added
S-blocked
|
☔ The latest upstream changes (presumably #97313) made this pull request unmergeable. Please resolve the merge conflicts. |
…r-errors Remove separate indexing of early-bound regions ~Based on rust-lang#99728 This PR copies some modifications from rust-lang#97839 around object lifetime defaults. These modifications allow to stop counting generic parameters during lifetime resolution, and rely on the indexing given by `rustc_typeck::collect`.
…r-errors Remove separate indexing of early-bound regions ~Based on rust-lang#99728 This PR copies some modifications from rust-lang#97839 around object lifetime defaults. These modifications allow to stop counting generic parameters during lifetime resolution, and rely on the indexing given by `rustc_typeck::collect`.
6 participants
Split from #97393
r? @jackh726