-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Resolve region bounds from components of type projection #121602
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
Resolve region bounds from components of type projection #121602
Conversation
This comment has been minimized.
This comment has been minimized.
☝️ The existing build failure is a perfect example that I would like quote in a question about the well-formedness clauses which I am wondering about. I am formulating my question to be posted later in Zulip. |
This should be assigned to T-types when it exits draft mode |
3e320bc
to
d6fdc9d
Compare
for &predicate in self.projection_predicates { | ||
let Some(ty::ProjectionPredicate { term, projection_ty }) = predicate.no_bound_vars() | ||
else { | ||
continue; |
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.
... so if we are here, we have, for now, T == for<'a, 'b, ...> <... as ...>::...
.
I guess it is safe to say that T: 'static
?
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.
I guess it is safe to say that T: 'static?
Why is that true?
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.
I retract. It is not necessarily.
This comment has been minimized.
This comment has been minimized.
d6fdc9d
to
0f8777e
Compare
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.
Thanks for the PR, but I'd personally lean towards closing this in favor of first having a discussion about whether we even want to support this at all.
Essentially this PR is a attempt to extend RFC 1214's projection rules (https://github.com/rust-lang/rfcs/blob/master/text/1214-projections-lifetimes-and-wf.md#outlives-for-projections) with a new rule:
TypeOutlivesViaBackwardsProjection:
∀i. R ⊢ Pi: 'a
<P0 as Trait<P1..Pn>>::Assoc normalizes-to T
--------------------------------------------------
R ⊢ Ty: 'a
In order to fix code that looks like:
trait Foo: Bar {
fn hello<'a>(&'a self) {}
}
trait Bar {
type Assoc;
}
// impl<T> Foo for T where T: Bar // WORKS
impl<T, A> Foo for T where T: Bar<Assoc = A> // BROKEN
{
fn hello<'a>(&'a self) {
outlives::<'a, Self::Assoc>();
}
}
fn outlives<'a, T: 'a>() {}
This is an attempt to fix a subset of a more general problem with the type system, where normalization only proceeds in one direction, even when considering an unnormalized type may allow us to prove goals that we cannot on the normalized type.
I don't believe this is unsound, as long as we're enforcing the same things that make RFC 1214's OutlivesProjectionComponents
sound (i.e. projection must not give us unconstrained lifetimes).
However, I'm still generally concerned that this doesn't interact well with higher-ranked projection bounds, and is still quite limited in the exact matching it can do (consider, e.g., a chain of two projection bounds), and I don't know if it's worth the extra maintenance burden to fix this specific case, since I assume it's hit only very rarely and can be worked around.
Side-note: It reminds me of another limitation where we do not consider alias bounds when we are able to project to a rigid type, even if that rigid type doesn't have the same assumptions:
trait Foo {
type Bar: Qux;
}
trait Qux {
fn hello();
}
fn doesnt_work<T: Foo<Bar = U>, U>() {
T::Bar::hello(); // <~ `T::Bar` becomes `U`, for which we know nothing.
}
fn works<T: Foo>() {
T::Bar::hello();
}
for &predicate in self.projection_predicates { | ||
let Some(ty::ProjectionPredicate { term, projection_ty }) = predicate.no_bound_vars() | ||
else { | ||
continue; |
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.
I guess it is safe to say that T: 'static?
Why is that true?
As I noted above, I'd like to close this since I don't believe the fix warrants the additional complications. If another T-types member would like to shepherd this work instead, feel free to cancel the FCP and take it over. @rfcbot fcp close |
Team member @compiler-errors has proposed to close this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
I think a good place to prototype this kind of change would be a-mir-formality. I'm not sure if we're quite there yet, but that's where the types team wants to get to when thinking about changes that fundamentally interact with type and borrow checking. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period, with a disposition to close, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. |
Fix #121601
This will stay in draft so as to resolve potential issue in design and approach, possible conflict with existing work on the type checker.