-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
parse and handle where T = U
predicate
#87471
Conversation
where T = U
predicate
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I think making it an actual PredicateKind would be better. You can use |
This comment has been minimized.
This comment has been minimized.
What's the status on the previous blockers? Have these been addressed now? |
It's not so clear to me whether the previous blockers are completed, but given the time that passed since then, I wouldn't be surprised if they have. I still need to clean up some of the branches (since I just threw them all in last night), and will explore if this is feasible. |
2ebff68
to
3b7341f
Compare
49baae2
to
a9a5025
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
5e385fb
to
a25ac5a
Compare
☔ The latest upstream changes (presumably #86735) made this pull request unmergeable. Please resolve the merge conflicts. |
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.
This could do with more tests (both successful and error cases). Also, could you add tests for the cases @nikomatsakis mentions in #22074 (comment)?
Since @nikomatsakis reviewed the original PR (#22074) that was closed, I'm going to reassign to them. |
af0231b
to
7695c47
Compare
match self.selcx.infcx().can_eq(obligation.param_env, lhs, rhs) { | ||
Ok(()) => ProcessResult::Changed(vec![]), | ||
Err(e) => ProcessResult::Error(FulfillmentErrorCode::CodeSubtypeError( | ||
ExpectedFound::new(false, lhs, rhs), |
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.
TODO create a new fulfillment error code
span, | ||
"type-equate requirement gave wrong error: `{:?}`", | ||
obligation | ||
) |
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.
Not entirely sure what this was referencing, I suspect it's fine to leave this?
@@ -327,6 +327,7 @@ impl<'a> Clean<Option<WherePredicate>> for ty::Predicate<'a> { | |||
ty::PredicateKind::RegionOutlives(pred) => pred.clean(cx), | |||
ty::PredicateKind::TypeOutlives(pred) => pred.clean(cx), | |||
ty::PredicateKind::Projection(pred) => Some(pred.clean(cx)), | |||
ty::PredicateKind::TypeEquate(..) => todo!(), |
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.
TODO not entirely sure whether to make this None
, or have something else
0900475
to
c2c011d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I don't think we really want this sort of where-clause. As I've come to learn more about the limits of various logics, introducing equality checks implies the ability to say things that we probably don't want to say (although I would like to support them someday, but only after we've made a transition to chalk and decided to do it). For example, being able to write |
☔ The latest upstream changes (presumably #87237) made this pull request unmergeable. Please resolve the merge conflicts. |
That's certainly fair, equality tends to imply Please let me know if there is additional work elsewhere I can do to help move specialization along! |
Updates #20041.
I took the most low-budget approach to adding equality checks (2 subtype checks in each direction), if it makes more sense to add in one check that does just equality, please let me know.
r? @varkor